Closed
Bug 1391486
Opened 7 years ago
Closed 7 years ago
Preference UI cloud storage option should handle browser.download.dir pref unspecified in new profile.
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
VERIFIED
FIXED
Firefox 57
People
(Reporter: pdahiya, Assigned: pdahiya)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
Gijs
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
Steps to replicate: Pr-requisite - Desktop with cloud storage provider client such as Dropbox 1. Open about:config and enable Cloud Storage API by setting pref ‘cloud.services.api.enabled’ to true 2. Set pref cloud.services.storage.key pref to provider key such as ’Dropbox’ 3. Ensure you have a valid download path for cloud storage downloads by manually creating folder ‘Downloads’ in provider folder e.g. ~/Dropbox/Downloads 4. Open about:preferences it should show radio button ‘Save to <provider name>' under Downloads and ‘Save to <system default downloads>’ first option as selected 5. Open about:config browser.download.folderList is set to 1 and browser.download.dir is unspecified 6. Selecting ‘Save to <provider name>' changes Download FileField label to ‘Desktop’ Expected: Download FileField label should stay ‘Downloads’
Assignee | ||
Comment 1•7 years ago
|
||
https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content-new/main.js#2487 From documentation above, we incorrectly assumed if browser.download.dir doesn’t exist or unspecified, browser.download.folderList is 0. It works till third option CloudStorage changes folderList value to 3 without touching browser.download.dir. In new profile, browser.download.dir is not specified and browser.download.folderList value is 1, we should handle that in preference UI with cloud storage option available.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → pdahiya
Assignee | ||
Comment 2•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8898557 -
Attachment is obsolete: true
Assignee | ||
Comment 4•7 years ago
|
||
Hi Gijs For new profile, starting default value for browser.download.folderList is 1, any subsequent change in folderList value sets browser.download.dir. With cloud storage option available, its possible to switch to folderList 3 without setting browser.download.dir causing this bug. Its a miss in Bug 1387153 and handling this in attached patch, please review. Thanks https://treeherder.mozilla.org/#/jobs?repo=try&revision=f568c633caceb02c63b59dc2bc2dd8dbb25987d8
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8898636 [details] Bug 1391486 - Cloud Storage - Handle browser.download.dir unspecified in new profile https://reviewboard.mozilla.org/r/170016/#review175214 Looks good, thanks for catching this!
Attachment #8898636 -
Flags: review?(gijskruitbosch+bugs) → review+
Pushed by pdahiya@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/549b756fc868 Cloud Storage - Handle browser.download.dir unspecified in new profile r=Gijs
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8898636 [details] Bug 1391486 - Cloud Storage - Handle browser.download.dir unspecified in new profile Approval Request Comment [Feature/Bug causing the regression]:Bug 1387153 [User impact if declined]: For new profiles, selecting cloud storage option under Download in preference UI will change Download File field folder label to ‘Desktop’ [Is this code covered by automated tests?]:Yes [Has the fix been verified in Nightly?]:Yes, local build [Needs manual test from QE? If yes, steps to reproduce]: Pr-requisite - Desktop with cloud storage provider client such as Dropbox Steps: 1. Open about:config and enable Cloud Storage API by setting pref ‘cloud.services.api.enabled’ to true 2. Set pref 'cloud.services.storage.key' to provider key such as ’Dropbox’ 3. Ensure you have a valid download path for cloud storage downloads by manually creating folder ‘Downloads’ in provider folder e.g. ~/Dropbox/Downloads 4. Open about:preferences it should show radio button ‘Save to <provider name>' under Downloads and ‘Save to <system default downloads>’ first option as selected 5. Open about:config browser.download.folderList is set to 1 and browser.download.dir is unspecified 6. Selecting ‘Save to <provider name>' should keep Download FileField label to ‘Downloads’ [List of other uplifts needed for the feature/fix]: Bug 1387153 . In the order first Bug 1387153 followed by Bug 1391486 fix [Is the change risky?]:No [Why is the change risky/not risky?]: Change hidden behind prefs and needed for shield study planned for 56 for a limited set of users. [String changes made/needed]:None
Attachment #8898636 -
Flags: approval-mozilla-beta?
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/549b756fc868
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•7 years ago
|
status-firefox56:
--- → affected
Comment 9•7 years ago
|
||
Comment on attachment 8898636 [details] Bug 1391486 - Cloud Storage - Handle browser.download.dir unspecified in new profile OK for uplift to beta. please land the work in bug 1387153 first.
Attachment #8898636 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/9f0bb349eced
Updated•7 years ago
|
Flags: qe-verify+
Comment 11•7 years ago
|
||
I can confirm that both 56.0b11 build1 (20170911193316) and 57.0a1 (2017-09-13) are fixed on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6. But there seems to be a side effect, reproducible on all above platforms and both mentioned builds: when switching between the third ("Always ask you where to save files") and the second option ("Save files to <provider name>") from the "Downloads" section, the first option ("Save files to <system default downloads>")gets selected for a very short time instead of the second one (see the screencast https://goo.gl/EbFQA2). This is reproducible once in a few attempts. Any thoughts about this?
Flags: needinfo?(pdahiya)
Assignee | ||
Comment 12•7 years ago
|
||
This fraction of seconds flicker happens when ‘Save to <provider>’ cloud storage option is selected by user under Downloads. Inside readCloudStorage, if browser.download.folderList pref satisfies value 3, we disable downloadFolder element that was just enabled in readUseDownloadDir. https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/main.js#2280 As noted in #Comment 7 above, ‘Save to <provider>’ option is hidden behind prefs and needed for shield study planned for 56 for a limited set of users (Bug 1399198). Since shield study is not testing preferences UI, my understanding is its a low risk bug. Thanks
Flags: needinfo?(pdahiya)
Comment 13•7 years ago
|
||
Based on comment 12 and comment 11, I will mark this as verified fixed, but file a separate bug for the glitch mentioned above.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•