1444521 - Preferences UI download folder and choose button stays disabled when save to cloud storage option is shown

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[Punam Dahiya [:pdahiya]]

Cloud Storage downloads preferences UI landed in bug 1387153 shows a second radio option 'save to <provider app>'

This feature is hidden behind prefs.  ‘cloud.services.api.enabled’ should be true to initialize API and ‘cloud.services.storage.key’ should be set to provider key to view cloud storage option under Downloads in about:preferences 

Steps to replicate the issue by manually creating pref and Downloads folder

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
5. Selecting the first radio option keeps the download folder and choose button stays disabled

Expected:
With first radio option selected, download folder and choose button should be enabled

Comment 1

[Punam Dahiya [:pdahiya]]

Debugging the issue, with bug 1379338 fix, eventlistener is not set and fails to invoke gMainPane.handleSaveToCommand  

Changing back to first radio option fails to change pref 'browser.download.FolderList' handled inside gMainPane.handleSaveToCommand, causing downloadfolder and choose button to stay disabled


https://hg.mozilla.org/mozilla-central/diff/0adedb70b788/browser/components/preferences/in-content/main.js#l1.238

Comment 2

[Punam Dahiya [:pdahiya]]

Feature is hidden behind pref and need not be tracked for 59, but it will be great to get fix in 60 for next cloud storage shield study.

Comment 3

[Sam Foster [:sfoster] (he/him)]

This seems reasonable and actionable, and I can put it in the backlog and it will get fixed eventually. But to get this uplifted to 60 this needs to land on central pretty soon and I'm not sure who would work on that. Punam, as the author of the original patch, do you have bandwidth to work on a patch for this?

Comment 4

[Punam Dahiya [:pdahiya]]

(In reply to Sam Foster [:sfoster] from comment #3)
> This seems reasonable and actionable, and I can put it in the backlog and it
> will get fixed eventually. But to get this uplifted to 60 this needs to land
> on central pretty soon and I'm not sure who would work on that. Punam, as
> the author of the original patch, do you have bandwidth to work on a patch
> for this?

yes, I will assign it to myself and submit patch for review this week. Thanks!

Comment 5

[Punam Dahiya [:pdahiya]]

Attachment: Bug 1444521 - Preferences UI save to cloud downloads event listener

Review commit: https://reviewboard.mozilla.org/r/227732/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/227732/

Comment 6

[Punam Dahiya [:pdahiya]]

Hi Jared

Attaching patch that’s a one line fix and adds back handleSaveToCommand as event listener. Please
review. Thanks!

Comment 7

[Punam Dahiya [:pdahiya]]

Link to try server

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1fc64f5ac22e1185046e4acce6f49f68e275bbe9

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8958852 [details]
Bug 1444521 - Preferences UI save to cloud downloads event listener

https://reviewboard.mozilla.org/r/227732/#review233744

Can you please file a follow-up bug to add tests for this UI? We should have some tests that make sure this keeps working as this is one example of it breaking silently, and the next time may go even longer before we notice. Normally I would ask that tests be included in the regression fix but I see you're trying to get this uplifted to 60 and I'm going on PTO for an extended weekend.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2cab87e3a891
Preferences UI save to cloud downloads event listener r=jaws

Comment 10

[Punam Dahiya [:pdahiya]]

(In reply to [away 3-15 to 3-18] Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Comment on attachment 8958852 [details]
> Bug 1444521 - Preferences UI save to cloud downloads event listener
> 
> https://reviewboard.mozilla.org/r/227732/#review233744
> 
> Can you please file a follow-up bug to add tests for this UI? We should have
> some tests that make sure this keeps working as this is one example of it
> breaking silently, and the next time may go even longer before we notice.
> Normally I would ask that tests be included in the regression fix but I see
> you're trying to get this uplifted to 60 and I'm going on PTO for an
> extended weekend.

Thanks Jaws for review, filed Bug 1446073 to add tests to avoid regressions.

Comment 11

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/2cab87e3a891

Comment 12

[Punam Dahiya [:pdahiya]]

Comment on attachment 8958852 [details]
Bug 1444521 - Preferences UI save to cloud downloads event listener

Approval Request Comment
[Feature/Bug causing the regression]:Bug 1379338
[User impact if declined]:
With 'Save files to <cloud provider>' second option visible, selecting options under Downloads keeps downloadfolder and choose button disabled

[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
Pr-requisite
- Desktop with cloud storage provider client such as Dropbox

Steps to test preference UI by manually creating pref and Downloads folder

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
5. On selecting first option, download folder and choose button stays disabled.

Expected: Selecting first option should enable download folder and choose button

[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]:No
[Why is the change risky/not risky?]: Feature is hidden behind prefs. One line fix that sets back event listener removed with fix of bug 1379338

[String changes made/needed]: No

Comment 13

[Julien Cristau [:jcristau]]

Are there planned experiments on 60 that will pref this on and require this fix, or can this safely ride the trains?

Comment 14

[Punam Dahiya [:pdahiya]]

(In reply to Julien Cristau [:jcristau] from comment #13)
> Are there planned experiments on 60 that will pref this on and require this
> fix, or can this safely ride the trains?

We are aiming to launch a second shield study en-US in April/May time frame, which will have a dependency of this fix in 60.
Thats why placed request to get it uplifted.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8958852 [details]
Bug 1444521 - Preferences UI save to cloud downloads event listener

thanks for that extra info; a=me for beta60

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6d9462961951

Comment 17

[roxleitan]

Verified fixed on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.12 on Nightly 61.0a1 (2018-03-22) and Beta 60.0b6.

Selecting the first option enables the download folder and browse button.
Selecting the "Save files to Dropbox" option disables the download folder and browse button.