Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileDialog does not handle relative file URIs well. #5234

Open
2 tasks done
steampoweredtaco opened this issue Nov 3, 2024 · 3 comments
Open
2 tasks done

FileDialog does not handle relative file URIs well. #5234

steampoweredtaco opened this issue Nov 3, 2024 · 3 comments
Labels
unverified A bug that has been reported but not verified

Comments

@steampoweredtaco
Copy link
Contributor

steampoweredtaco commented Nov 3, 2024

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

When using a relative directory to file dialog's SetLocation method and it is a relative directory it will not produce any contents. If you select the listing icon then files will show, but not the breadcrumb buttons. You can then click grid again to see the files, but again no breadcrumb buttons.

How to reproduce

Simply create a file dialog and set the storage uri to a relative directory that exists to the running working directory of the program. Then click the list/grid buttons to see behaviors as described.

Screenshots

This is an example of what the bug looks like:
___1go_build_ttstick_runtimes_traceReader_x1Wtl3O6WC

Gif showing that the file dialog still sorta works when you hit the list/grid buttons after but due to the nature of this issue the breadcrumb buttons will not be available:
___go_build_ttstick_runtimes_traceReader_S6exROEjpN

The workaround is simple of course, first make sure the directory is an absolute directory before setting the location:
___go_build_ttstick_runtimes_traceReader_t4d10mk6Lc

Example code

Of course make sure ./logs exist with some files from the runtime directory when the program starts for this example to work.

package main

import (
	"context"
	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/dialog"
	"fyne.io/fyne/v2/storage"
)

func main() {
	_, cancel := context.WithCancel(context.Background())
	a := app.New()
	w := a.NewWindow("Trace Reader")
	w.Resize(fyne.NewSize(800, 600))
	w.SetOnClosed(cancel)

	var fileOpener *dialog.FileDialog
	fileOpener = dialog.NewFileOpen(func(reader fyne.URIReadCloser, err error) {
		if err != nil {
			dialog.ShowError(err, w)
		}
	}, w)

	p := "./logs"
	// Workaround is as follows
	// p, err := filepath.Abs(p)
	// if err != nil {
	// 	panic(err)
	// }
	u := storage.NewFileURI(p)
	d, err := storage.ListerForURI(u)
	if err != nil {
		panic(err)
	}
	fileOpener.SetLocation(d)
	fileOpener.Show()
	w.SetMaster()
	w.ShowAndRun()
}

Fyne version

v2.5.2

Go compiler version

1.23.1

Operating system and version

Windows 10

Additional Information

I tracked down this issue to the code that creates the breadcrumbs loop
/fyne/dialog/file.go:468
image
This will take the location "./logs" and end up parsing it to "/" then "/logs" which does not exist. so the isDir checks fail.

Although the workaround is simple, the fact fyne doesn't return an error wit SetLocation there is no way from a client code perspective to know there was a problem at runtime (setLocation does return an error, but SetLocation does not).

I think if an error was returned from SetLocation stating it does not support relative file uri locations that would of been fine.

I suspect this should work though instead of throwing an error, but maybe file dialog needs a way to recognize if the storage provider url support an absolute function of some sort and use that before creating the bread crumbs?

Also, given the way the bread crumbs are made, I suspect this bug hits in other scenarios like when the user has permissions to list in the directory they opened and navigated in but in the parent paths they do not have listing permissions which is a possible edge case in some OS's. That kind of issue would manifest in the same way. It might make more sense if the breadcrumbs were made from the current directory up to parents instead of starting with the root parent; in this way a partial bread crumb could be listed at all times, even if it is only for the current directory, up until the path breaks.

@steampoweredtaco steampoweredtaco added the unverified A bug that has been reported but not verified label Nov 3, 2024
@andydotxyz
Copy link
Member

We do not need to add support for checking a URI for absolute. A URI must be absolute, there is no support for relative in the way URI is parsed or handled.

It may be a useful improvement for the "NewFileURI" to ensure the path is absolute before passing it to the URI code.

@andydotxyz
Copy link
Member

The change you suggest to breadcrumbs is already underway in an open PR.

@steampoweredtaco
Copy link
Contributor Author

We do not need to add support for checking a URI for absolute. A URI must be absolute, there is no support for relative in the way URI is parsed or handled.

It may be a useful improvement for the "NewFileURI" to ensure the path is absolute before passing it to the URI code.

Yes, if the intention for fyne all storage path URIs are absolute it would make sense if new file uri normalized it to be absolute to the working directory for Windows, not sure the answer for other platforms. That, or throwing an error, would of saved me time.

The interface doc probably should also mention that this normalization to absolute even if the URI was created with a relative path must happen if components in fyne will require it at all times. (Probably overkill to make a new AbsoluteUri interface/method).

Feel free to change the title of this issue to better reflect this is more of a bug related to NewFileUri or close it in favor of a new issue if you want.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unverified A bug that has been reported but not verified
Projects
None yet
Development

No branches or pull requests

2 participants