Open Bug 1799381 Opened 2 years ago Updated 1 year ago

Snap version ignores "savePerSite" function

Categories

(Firefox Build System :: Third Party Packaging, defect, P4)

Firefox 106
defect

Tracking

(Not tracked)

UNCONFIRMED

People

(Reporter: grzegorz, Unassigned)

References

(Blocks 1 open bug)

Details

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:106.0) Gecko/20100101 Firefox/106.0

Steps to reproduce:

Firefox 106 snap version in Kubuntu 22.10.

I have savePerSite preference enabled. That's how it worked:

  1. I go to a website example.com, try to upload a file, file picker opens on the last location I used with this website: /home/user/example
  2. I go to a website different-example.com, try to upload a file, file picker opens on the last location I used with this website: /home/user/different-example
  3. I go back to example.com, file picker again opens on /home/user/example

Actual results:

With snap version of Firefox the file picker always opens on last location. It does not remember per-site locations.

  1. I go to a website example.com, try to upload a file, file picker opens on the last location I used with this website: /home/user/example
  2. I go to a website different-example.com, try to upload a file, file picker opens on the last location I used with a previous website: /home/user/example, I manually browse to /home/user/different-example
  3. I go back to example.com, file picker opens on /home/user/different-example - it does not remember per-site settings

Expected results:

Firefox should remember the disk location I used with different websites.

The Bugbug bot thinks this bug should belong to the 'Firefox Build System::General' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → General
Product: Firefox → Firefox Build System
Component: General → Downloads API
Product: Firefox Build System → Toolkit

I expect the issue is that although we remember the path, we cannot open the filepicker at that location anymore. I don't know that there's any way around that - certainly not across restarts, but even in a single process, we'd presumably permanently need to keep a file handle lying around for reuse for each site, or something. IIRC we use a sqlite database and store paths in there, so we'd have to have some extra in-memory mapping and even that wouldn't work across restarts. I think this is fundamentally an issue with snap.

Component: Downloads API → Widget: Gtk
Product: Toolkit → Core

(In reply to :Gijs (he/him) from comment #2)

I expect the issue is that although we remember the path, we cannot open the filepicker at that location anymore. I don't know that there's any way around that - certainly not across restarts, but even in a single process, we'd presumably permanently need to keep a file handle lying around for reuse for each site, or something. IIRC we use a sqlite database and store paths in there, so we'd have to have some extra in-memory mapping and even that wouldn't work across restarts. I think this is fundamentally an issue with snap.

The path that Gecko will get is a XDG Document Portal one, something of the form /run/..., is it possible since it is different from the one mentionned this is just making the feature fail?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Alexandre LISSY :gerard-majax from comment #3)

(In reply to :Gijs (he/him) from comment #2)

I expect the issue is that although we remember the path, we cannot open the filepicker at that location anymore. I don't know that there's any way around that - certainly not across restarts, but even in a single process, we'd presumably permanently need to keep a file handle lying around for reuse for each site, or something. IIRC we use a sqlite database and store paths in there, so we'd have to have some extra in-memory mapping and even that wouldn't work across restarts. I think this is fundamentally an issue with snap.

The path that Gecko will get is a XDG Document Portal one, something of the form /run/..., is it possible since it is different from the one mentionned this is just making the feature fail?

That's quite possible, but that's still a snap issue. I don't see how the downloads code could reasonably work around that. Snap needs a way to permanently keep the permissions on the folders where the user has said they want to save files, rather than the temporary XDG aliases. Am I missing something?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(lissyx+mozillians)

I never implied otherwise, I just want to make sure of the root cause, because in this case it would just be a dupe of https://bugzilla.mozilla.org/show_bug.cgi?id=1775497

Flags: needinfo?(lissyx+mozillians)

(In reply to Alexandre LISSY :gerard-majax from comment #5)

I never implied otherwise, I just want to make sure of the root cause, because in this case it would just be a dupe of https://bugzilla.mozilla.org/show_bug.cgi?id=1775497

Ah, hm - maybe? I don't know enough about how snap works to judge that. All I can tell is it breaks code that worked for the best part of 2 decades and I don't know how to fix it. :-(

Severity: -- → S3
Component: Widget: Gtk → Third Party Packaging
Priority: -- → P4
Product: Core → Firefox Build System
Duplicate of this bug: 1815043

locally apply https://github.com/flatpak/xdg-desktop-portal/pull/1007 and verify if it fixes as we hope

Flags: needinfo?(lissyx+mozillians)

Wrong bug.

Flags: needinfo?(lissyx+mozillians)

(In reply to :Gijs (he/him) from comment #2)

I expect the issue is that although we remember the path, we cannot open the filepicker at that location anymore. I don't know that there's any way around that - certainly not across restarts, but even in a single process, we'd presumably permanently need to keep a file handle lying around for reuse for each site, or something. IIRC we use a sqlite database and store paths in there, so we'd have to have some extra in-memory mapping and even that wouldn't work across restarts. I think this is fundamentally an issue with snap.

So now we can get bug 1775497 unblocked, and browser.download.lastDir is still the /run/xxx path but XDG Desktop Portal will know to show it as e.g., /tmp.

I am however unable to understand how savePerSites is supposed to work (I dont even get it to work outside of snap), and I failed to find proper doc.

Flags: needinfo?(gijskruitbosch+bugs)

I am also a bit troubled by the bug report ; re-reading, it mentions uploading files, while savePerSite seems to be only about downloading ...

I'm not sure how this functionality is called, but in non-snap version of Firefox it works with uploads. I thought it's savePerSite.

It's especially useful with uploads as well, since it allows Firefox to remember that for photo sharing website I always open my photo folder, for government tax website - my tax folder, and so on. Some of those sites, like the tax one for example, are used only once per year so it's very convenient that Firefox remembers the upload location for each site.

(In reply to Alexandre LISSY :gerard-majax from comment #10)

I am however unable to understand how savePerSites is supposed to work (I dont even get it to work outside of snap), and I failed to find proper doc.

I'm not sure what the question is.

My understanding of how it's supposed to work is:

  • set Firefox to always ask you where to save files, rather than always saving to the set download folder
  • download file from site A. Save to folder A.
  • download file from site B. Save to folder B.
  • if you now download again from site A, the filepicker should start off in folder A. Ditto for site B / folder B.

The implementation is in https://searchfox.org/mozilla-central/source/toolkit/mozapps/downloads/DownloadLastDir.sys.mjs, which also has a doc comment at the top that mentions some subtleties around private browsing etc. For the domain, it uses the referrer URI, or source URI if no referrer URI is available. A typical callsite is https://searchfox.org/mozilla-central/rev/0c2945ad4769e2d4428c72e6ddd78d60eb920394/toolkit/content/contentAreaUtils.js#679 or https://searchfox.org/mozilla-central/rev/0c2945ad4769e2d4428c72e6ddd78d60eb920394/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs#226,343 .

Does that help? If not, can you say more about what you're trying and seeing vs. expecting?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(lissyx+mozillians)

Well this is what I tested, and it was not working on my side, Snap or not. Maybe I did a mistake, I'll have to retest.

Flags: needinfo?(lissyx+mozillians)
You need to log in before you can comment on or make changes to this bug.