fyne-io / fyne

Cross platform GUI toolkit in Go inspired by Material Design
https://fyne.io/
Other
25.26k stars 1.4k forks source link

Bug in dialog.NewFolderOpen() with custom repositories #5200

Closed brucealthompson closed 1 month ago

brucealthompson commented 1 month ago

Checklist

Describe the bug

Register a new repository with a new scheme. I used the scheme "httpfile". Then set the folder location for dialog.NewFolderOpen() with the following code:

locationURI, err := storage.ParseURI("httpfile:///")
  if err != nil {
    return err
  }
  listableURI, err := storage.ListerForURI(locationURI)
  if err != nil {
    return err
  }
  folderdialog := dialog.NewFolderOpen(callback, parent)
  folderdialog.SetLocation(listableURI)
  folderdialog.Show()

The resulting file dialog uses the default directory (without returning an error) instead of the URI in listableURI. func (f *FileDialog) effectiveStartingDir() fyne.ListableURI in fyne.io\fyne\v2@v2.5.2\dialog\file.go has incorrect logic at the beginning.

I have already found the code causing this issue and fixed it. Here is the code causing this issue in: fyne.io\fyne\v2@v2.5.2\dialog\file.go

func (f *FileDialog) effectiveStartingDir() fyne.ListableURI {
    if f.startingLocation != nil {
        if f.startingLocation.Scheme() == "file" {
            path := f.startingLocation.Path()

            // the starting directory is set explicitly
            if _, err := os.Stat(path); err != nil {
                fyne.LogError("Error with StartingLocation", err)
            } else {
                return f.startingLocation
            }
        }

    }

Here is the fix:

func (f *FileDialog) effectiveStartingDir() fyne.ListableURI {
    if f.startingLocation != nil {
        if f.startingLocation.Scheme() == "file" {
            path := f.startingLocation.Path()

            // the starting directory is set explicitly
            if _, err := os.Stat(path); err != nil {
                fyne.LogError("Error with StartingLocation", err)
            } else {
                return f.startingLocation
            }
        }
        **return f.startingLocation**
    }

Add return f.startingLocation to the end of the above code segment.

How to reproduce

Register a new repository with a new scheme. I used the scheme "httpfile". Then set the folder location for dialog.NewFolderOpen() with the following code:

locationURI, err := storage.ParseURI("httpfile:///")
  if err != nil {
    return err
  }
  listableURI, err := storage.ListerForURI(locationURI)
  if err != nil {
    return err
  }
  folderdialog := dialog.NewFolderOpen(callback, parent)
  folderdialog.SetLocation(listableURI)
  folderdialog.Show()

The resulting file dialog uses the default directory (without returning an error) instead of the URI in listableURI. func (f *FileDialog) effectiveStartingDir() fyne.ListableURI in fyne.io\fyne\v2@v2.5.2\dialog\file.go has incorrect logic at the beginning.

Screenshots

No response

Example code

locationURI, err := storage.ParseURI("httpfile:///")
  if err != nil {
    return err
  }
  listableURI, err := storage.ListerForURI(locationURI)
  if err != nil {
    return err
  }
  folderdialog := dialog.NewFolderOpen(callback, parent)
  folderdialog.SetLocation(listableURI)
  folderdialog.Show()

Fyne version

2.5.2

Go compiler version

1.23.1

Operating system and version

Windows 11

Additional Information

I have already implemented the fix in my copy of fyne. See above for the fix. Works fyne now.

andydotxyz commented 1 month ago

Thanks for the careful analysis - I agree with the proposed fix. Would you like to place that into a PR so it gets associated with your account? :). (plus Hacktoberfest credits!)

brucealthompson commented 1 month ago

Would you like to place that into a PR so it gets associated with your account? :). (plus Hacktoberfest credits!)

PR == Merge Request?? Can you point me to documentation for your development process? I assume you want me to check into a branch off of main on Github. Is there any naming convention you use?

andydotxyz commented 1 month ago

PR = Pull Request, you would fork the repo, make a branch from develop with whatever name you want, then push it to your fork. Go to GitHub and open a pull request for us to look at the change.

That said if it's a tiny change you might get away with the GitHub "edit file" feature which automates a lot of that for you.

brucealthompson commented 1 month ago

Created the pull request: https://github.com/fyne-io/fyne/pull/5213

andydotxyz commented 1 month ago

Thanks so much for the fix :)