1742146 - Unable to change the download folder - (you need to change it twice)

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Pablo]

Affected platforms
Windows 10
MacOS 11

Affected versions
Nightly 96.0a1
Beta 95.0b9
Release 94.0.1

Steps to reproduce
launch firefox with new profile
go to about preferences
search for "download"
on download folder, click "browse" button
click desktop
double click any folder that you have on your desktop
click on "select folder" button

Actual Results
Folder stays as download

Expected Results
Folder should change to whatever folder you chose.

Note
if you try same steps a second time, then firefox takes the folder name correctly in about:preferences.

Comment 1

[:Gijs (he/him)]

[Tracking Requested - why for this release]:
Not respecting user preferences is bad and this UX is really bad, and it isn't obvious that doing it a second time works.

I'm tracking down a regression range, but off-hand it would seem esr91 was also affected. Pablo, can you confirm? Also, was Linux not affected or did you just not have time/opportunity to test on Linux?

Comment 2

[:Gijs (he/him)]

Err, sorry, think I was mistaken about esr91 - though it'd be good to have confirmation either way.

Comment 3

[:Gijs (he/him)]

Regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e4b2a561a96fd31b8615980fdaafac616acfbc74&tochange=120f83c79d5c22ea8cf344bd901de79333dc035b

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1742146 - don't race localization against non-localization when updating the downloads prefs, r?jaws

This race condition happens because some of the paths through displayDownloadDirPrefTask
are synchronous, and some are not, and we repeatedly call it in the same turn of the
event loop, both because downloads prefs are stored in 2 prefs, both of which impact
this display, and because the pref bindings like calling 'change' event listeners more
than once.

To make this reliably deterministic, instead of juggling callsites, just ensure that
the last caller to displayDownloadDirPrefTask is always the one that actually
determines the label that gets displayed.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1613705

Comment 6

[Pablo]

Hi gijs,

ESR 91.3.0 is not affected, changing the download folder works on windows10, Mac 11 and Ubuntu 20.

The issue also affects Ubuntu, but i didn´t have a chance to test it since my multiboot was broken.
I was able to fix it this weekend and it affects all latest nightly, latest beta, and release 94.0.2

Comment 7

[:Gijs (he/him)]

Thanks!

I'll note that behind the scenes, the internal preference does change the first time you select a folder, but the UI doesn't update correctly, which is doubly confusing...

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f03340a5c17d
don't race localization against non-localization when updating the downloads prefs, r=jaws,preferences-reviewers

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f03340a5c17d

Comment 10

[:Gijs (he/him)]

Comment on attachment 9252008 [details]
Bug 1742146 - don't race localization against non-localization when updating the downloads prefs, r?jaws

Beta/Release Uplift Approval Request

• User impact if declined: Confusing experience trying to change the downloads folder
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very localized JS-only change to remove a race condition responsible for the confusion
• String changes made/needed: None

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9252008 [details]
Bug 1742146 - don't race localization against non-localization when updating the downloads prefs, r?jaws

Low risk, approved for our last 95 beta, thanks.

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/9a398d07f282

Comment 13

[Pablo]

Verified fix on latest Firefox Nightly version 96.0a1 using Windows10 64 bit, MacOS 11 , and Ubuntu 20.
I was able to change the folder at first try

Comment 14

[Pablo]

Verified fix on beta 95.0b12 using Windows10 64 bit, MacOS 11 , and Ubuntu 20.