Open Bug 1400154 Opened 2 years ago Updated 2 years ago

Preference UI cloud storage option flicker

Categories

(Firefox :: Preferences, defect, P5, minor)

defect

Tracking

()

ASSIGNED
Tracking Status
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix

People

(Reporter: JuliaC, Assigned: pdahiya)

References

Details

(Keywords: regression)

Attachments

(1 file)

[Note]:
- See https://bugzilla.mozilla.org/show_bug.cgi?id=1391486#c11 and https://bugzilla.mozilla.org/show_bug.cgi?id=1391486#c12 for background

[Affected versions]:
- 56.0b11 build1 (20170911193316)
- 57.0a1 (2017-09-14)

[Affected platforms]:
- Windows 10 x64
- Ubuntu 16.04 x64
- Mac OS X 10.11.6

[Steps to reproduce]:
Prerequisite:
- Cloud storage provider client desktop app installed, such as Dropbox

1. Launch Firefox with a clean profile
2. Open about:config and create "cloud.services.api.enabled" boolean pref (set its value to "true") and "cloud.services.storage.key" string pref (set its value to a provider key such as "Dropbox")
3. Ensure you have a valid download path for the cloud storage downloads by manually creating the "Downloads" folder in the provider's folder e.g. ~/Dropbox/Downloads
4. Open about:preferences, switch between the 3 options under the "Downloads" section and inspect each corresponding radio button 

[Expected result]:
- The user is able to easily switch between each download option 
- Each corresponding radio button is fully functional

[Actual result]:
- 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).

[Regression range]:
- This issue is triggered by bug 1391486
Marking wontfix for 58 since we are still doing a shield study (bug 1391486) to learn more about this.

Punam, can you work on fixing this? From a brief look at the code I think it might be related to your use of await.
Flags: needinfo?(pdahiya)
Assignee: nobody → pdahiya
Flags: needinfo?(pdahiya)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Marking wontfix for 58 since we are still doing a shield study (bug 1391486)
> to learn more about this.
> 
> Punam, can you work on fixing this? From a brief look at the code I think it
> might be related to your use of await.

Yes, assigned myself to look into it and submit fix Thanks!
Hi Jared

I have tried fixing the flicker issue by explicitly storing the radio option selected when switching away from alwaysAsk option and handling that in browser.download.useDownloadDir onsyncfrompreference handler. Please review

Link to try server

https://treeherder.mozilla.org/#/jobs?repo=try&revision=228b0cec4905f75f6ffaa464b7b0f202123f6f4c

Thanks!
Comment on attachment 8928044 [details]
Bug 1400154 - Preferences UI download option flicker

https://reviewboard.mozilla.org/r/199274/#review207464

This seems very complex for something that shouldn't need to be. When the user clicks on the radio button, why are we changing the value again once useDownloadDir becomes true? It seems we may be doing more work than necessary.
Attachment #8928044 - Flags: review?(jaws) → review-
Comment on attachment 8928044 [details]
Bug 1400154 - Preferences UI download option flicker

https://reviewboard.mozilla.org/r/199274/#review208562

::: browser/components/preferences/in-content/main.js:2370
(Diff revision 1)
>  
> +      // If user has switched away from alwaysAsk radio option
> +      // setting browser.download.useDownloadDir to true, check for
> +      // _useDownloadDirOption to get id of radio option selected
> +      // and set folderListPref with respective value
> +      if (this._useDownloadDirOption) {

saveToCloud and saveTo radio options has browser.download.useDownloadDir preference set to true. When user switches back from alwaysAsk to useDownloadDir as true we need to check which option is selected and set the folderListPref value accordingly

With CloudStorage download option available, there are two ways folderList pref value can change

a) switching between SaveTo and SaveToCloud radio option. This is handled in handleSaveToCommand

b)switching from alwaysAsk radio option to either SaveTo or SaveToCloud, this is handled here inside onsyncfromprefernce handler.
https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/main.xul#660

Failing to explicitly handle this case here plus use of await with getProviderifInUse on every onsyncfrompreference handling, I believe is the reason we see flicker.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> Comment on attachment 8928044 [details]
> Bug 1400154 - Preferences UI download option flicker
> 
> https://reviewboard.mozilla.org/r/199274/#review207464
> 
> This seems very complex for something that shouldn't need to be. When the
> user clicks on the radio button, why are we changing the value again once
> useDownloadDir becomes true? It seems we may be doing more work than
> necessary.

Tried answering your query above, please let me know if it can be refactored simpler. Thanks
Flags: needinfo?(jaws)
It was great to meet you in person Punam. To put our notes in this bug, Gijs and I met with Punam in Austin and discussed this bug. We would like to hold off on fixing this while we wait to get more data from the Shield study. This bug, while noticeable and "janky", does not affect the usability of the feature and requires the user to follow some steps that are not very common.

Since there is not a simple straight-forward fix for this, let's leave this bug on file and if we determine that we will move forward with this UI we can look at restructuring how it works so we will not need such a complex fix for this.
Flags: needinfo?(jaws)
You need to log in before you can comment on or make changes to this bug.