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

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

Closed
2 tasks done
brucealthompson opened this issue Oct 15, 2024 · 5 comments
Closed
2 tasks done

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

brucealthompson opened this issue Oct 15, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@brucealthompson
Copy link
Contributor

brucealthompson commented Oct 15, 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

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.

@brucealthompson brucealthompson added the unverified A bug that has been reported but not verified label Oct 15, 2024
@andydotxyz
Copy link
Member

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!)

@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Oct 15, 2024
@andydotxyz andydotxyz added this to the "F" release, Late 2024 milestone Oct 15, 2024
@brucealthompson
Copy link
Contributor Author

brucealthompson commented Oct 16, 2024

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
Copy link
Member

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
Copy link
Contributor Author

brucealthompson commented Oct 18, 2024

Created the pull request:
#5213

@andydotxyz
Copy link
Member

Thanks so much for the fix :)

andydotxyz pushed a commit that referenced this issue Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants