1441949 - Cloud Storage Downloads Shield Study Phase 2 Web Extension

Status: NEW

(Firefox :: Downloads Panel, defect, P3)

Description

[Punam Dahiya [:pdahiya]]

Implement notifications revised UI based off feedback from cloud downloads shield study. In cloud downloads second phase, notification prompting user to save downloads is moved to download panel.

- User can opt-in to saving subsequent downloads to cloud storage provider folder by clicking 'save to <provider>'.

- Support multiple storage providers

- Sync messaging to support Firefox Account user

- Links to about:preferences to help educate user change download settings.

Notification mockups on Screen 2 and 3 - 
https://mozilla.invisionapp.com/share/PKFN61KFZ25#/screens/278884758

Comment 1

[Punam Dahiya [:pdahiya]]

Hi Seevan

Working on implementing notifications in download panel. NI you to provide

a) download arrow svg when multiple provider exists and
b) hover color for Save and Cancel buttons.

Thanks!

Comment 2

[Punam Dahiya [:pdahiya]]

Attachment: WIP Cloud Storage Download Notification UI Patch

Comment 3

[Sevaan Franks [:sevaan]]

Attachment: download-16.svg

> a) download arrow svg when multiple provider exists and

Icon attached.

> b) hover color for Save and Cancel buttons.

Color information can be found here: https://design.firefox.com/photon/components/buttons.html

# Blue Buttons:

Default Background: Blue 60 #0060df

Hover Background: Blue 70 #003eaa

Pressed Background: Blue 80 #002275

Focus: box-shadow: 0 0 0 1px #0a84ff inset, 0 0 0 1px #0a84ff, 0 0 0 4px rgba(10, 132, 255, 0.3)

Disabled: 40% Opacity


# Grey Buttons:

Default Background: Grey 90 a10 rgba(12, 12, 13, 0.1)

Hover Background: Grey 90 a20 rgba(12, 12, 13, 0.2)

Pressed Background: Grey 90 a30 rgba(12, 12, 13, 0.3)

Focus: box-shadow: 0 0 0 1px #0a84ff inset, 0 0 0 1px #0a84ff, 0 0 0 4px rgba(10, 132, 255, 0.3)

Disabled: 40% Opacity

Comment 4

[Punam Dahiya [:pdahiya]]

Notification UI to be tested as phase 2 shield study as scoped in bug 1450016.

Implemented attached patch Notification UI as web extension in GitHub below

https://github.com/mozilla/shield-cloudstorage/tree/cloud-storage-web-ext

Comment 5

[Punam Dahiya [:pdahiya]]

Updating bug title to reflect cloud storage shield study phase 2 web extension development as scoped in bug 1450016 which has both

- Revised Notification UI
- Move to Cloud Storage <Provider> context menu support in Download Panel (Bug 1447521)

Link to updated mockups:
https://mozilla.invisionapp.com/share/PKFN61KFZ25#/screens/290895621

Comment 7

[Punam Dahiya [:pdahiya]]

Attachment: cloudstorage-webextensionexperiment_shield.mozilla.org-1.0.0.zip

Attaching cloud storage phase 2 web extension. Set pref "xpinstall.signatures.required" boolean pref to "false" to install and test extension in nightly

Comment 8

[Punam Dahiya [:pdahiya]]

Attachment: cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip

Comment 9

[Punam Dahiya [:pdahiya]]

Comment on attachment 8970338 [details]
cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip

Hi Sevaan

Requesting UI review on cloud storage web extension for en-US 60 shield study. Please let me know if you need specific screenshots to help review edge cases. Thanks!

Comment 10

[Punam Dahiya [:pdahiya]]

Attachment: Cloud Storage UI Screenshots

Hi Sevaan
As discussed, please find attached screenshots of cloud storage phase 2 extension for UI review. Thanks!

Comment 11

[Sevaan Franks [:sevaan]]

Comment on attachment 8972136 [details]
Cloud Storage UI Screenshots

In the context menu when right-clicking on a file that is already in cloud storage it says "Move to Local Download", but I think it should be "Move to Local Downloads" with an S on the end, since it's talking about the folder.

+r with that correction.

Comment 12

[Punam Dahiya [:pdahiya]]

Thanks Sevaan for UI review! One quick question on icon, we are using attached download.svg as icon for web extension

https://github.com/mozilla/shield-cloudstorage/blob/cloud-storage-web-ext/webextension/manifest.json#L28

Is that good or do we need a new one?

Comment 13

[Sevaan Franks [:sevaan]]

Attachment: sync2cloud.svg

Let's use this attachment for the web extension icon. It's a little more fun!

Comment 14

[Punam Dahiya [:pdahiya]]

Attachment: cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip

Hi Gijs

You have reviewed cloud-storage shield study code in GitHub in PRs
https://github.com/mozilla/shield-cloudstorage/pull/31
https://github.com/mozilla/shield-cloudstorage/pull/2

Since then, study was waiting on shield-utils v5. Integration with v5 PR is done and reviewed by Motin and extension is in QA now. Setting review flag to track peer review in the bug. Thanks!

Comment 15

[:Gijs (he/him)]

(In reply to Punam Dahiya [:pdahiya] from comment #14)
> Created attachment 8986003 [details]
> cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip
> 
> Hi Gijs
> 
> You have reviewed cloud-storage shield study code in GitHub in PRs
> https://github.com/mozilla/shield-cloudstorage/pull/31
> https://github.com/mozilla/shield-cloudstorage/pull/2
> 
> Since then, study was waiting on shield-utils v5. Integration with v5 PR is
> done and reviewed by Motin and extension is in QA now. Setting review flag
> to track peer review in the bug. Thanks!

I'm confused. What am I supposed to review at this point? If this is the same code that was reviewed in github, I don't think there's any point reviewing it again. If it isn't, can you provide a diff of what changed rather than just a zip file?

Comment 16

[Punam Dahiya [:pdahiya]]

(In reply to :Gijs (he/him) from comment #15)
> (In reply to Punam Dahiya [:pdahiya] from comment #14)
> > Created attachment 8986003 [details]
> > cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip
> > 
> > Hi Gijs
> > 
> > You have reviewed cloud-storage shield study code in GitHub in PRs
> > https://github.com/mozilla/shield-cloudstorage/pull/31
> > https://github.com/mozilla/shield-cloudstorage/pull/2
> > 
> > Since then, study was waiting on shield-utils v5. Integration with v5 PR is
> > done and reviewed by Motin and extension is in QA now. Setting review flag
> > to track peer review in the bug. Thanks!
> 
> I'm confused. What am I supposed to review at this point? If this is the
> same code that was reviewed in github, I don't think there's any point
> reviewing it again. If it isn't, can you provide a diff of what changed
> rather than just a zip file?

It's the same code, commits post your review are mainly integrating v5 shield utils (reviewed by motin) in the extension.
https://github.com/mozilla/shield-cloudstorage/commits/master


The ask for shield study launch is to get peer review in Bugzilla for tracking purpose. I wasn't sure how to set the review flag for a github code in bug, that's why used zip file of the extension. A peer review + as a comment should work as well. Thanks!

Comment 17

[:Gijs (he/him)]

Comment on attachment 8986003 [details]
cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip

I don't think carrying over review from github in this way is useful. I'm happy to sign off on whatever code I reviewed in github already, but I haven't tested or reviewed this zip file in any way, so it feels weird to be asked for review on it. I'm not sure what "tracking purpose" is helped by me setting this flag. In general, on bugzilla, you could just have set the review flag to "+" yourself and commented that I and/or other peers had reviewed this on github, if there were really no substantive changes from that point.

Comment 18

[Punam Dahiya [:pdahiya]]

Thanks Gijs, we are still laying down these processes, agree going forward review + by the developer commenting and linking to peer reviewed PR in GitHub should suffice.

Comment 19

[Punam Dahiya [:pdahiya]]

Attachment: cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.0.zip for signing

Comment 20

[Punam Dahiya [:pdahiya]]

Hi Mike

Attaching the extension for signing so that it can be submitted for final QA approval

Link to code in GitHub
https://github.com/mozilla/shield-cloudstorage


Link to PHD attached to bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1450016

Will appreciate if you can help sign attached extension. Thanks!

Comment 21

[Michael Cooper [:mythmon]]

Attachment: [e-mail]

Comment 22

[Punam Dahiya [:pdahiya]]

Hi Gregg, Motin

NI you to give your r+ in the comment on shield utils v5.0.2 integration code inside cloud storage extension

https://github.com/mozilla/shield-cloudstorage

Thanks!

Comment 23

[Gregg Lind (Fx Strategy and Insights - Shield - Heartbeat )]

R+ from me.

Comment 24

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Hi guys,

We have finished testing the latest Cloud Storage Shield Study (v5.0.2) experiment.

QA’s recommendation: GREEN - SHIP IT

Reasoning:
Both the survey issue (https://github.com/mozilla/shield-studies-addon-utils/issues/203) and the expiration pref reset issue (https://github.com/mozilla/shield-cloudstorage/issues/49) that were considered as blockers, were fixed and verified on the latest Cloud Storage (5.0.2) version. Also, during testing we haven't found any new issues.

Testing Summary:
- Full Functional test suite: TestRail;
- Verified that the Telemetry pings are correctly sent;
- Performed regression testing after reported issues were fixed;

Tested Platforms:
- Windows 10 x64;
- Windows 7 x64;
- macOS 10.13;
- Ubuntu 16.04 x64;

Tested Firefox versions:
- Firefox 60.0.2 release

Comment 25

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Hi guys,

We just finished testing Cloud Storage (v5.0.2) version on latest Firefox 61.0 release on tree different OSes (Windows 10x64, Mac 10.13 and Ubuntu 16.04 x64). During testing we haven't found any new issues.

Also, since the fix for issue (https://github.com/mozilla/shield-cloudstorage/issues/51) is not yet merged into master, we have created a build from Punam's unmerged branch which contained the fix and the issue is no longer reproducible. However, just to be sure we will reverify the issue after it will be merged into master.

Considering this, after the fix for issue #51 will be merged from the QA point of view the add-on is ready to be released to end users.

Comment 26

[Punam Dahiya [:pdahiya]]

Attachment: cloud_storage_shield_study_web_extension_experiment_cssswee_-1.0.1.zip for signing

Comment 27

[Punam Dahiya [:pdahiya]]

Hi Mike

Attaching the updated extension 1.0.1 with fix of #51. Will appreciate if you can help sign attached extension. Thanks!

Comment 28

[Michael Cooper [:mythmon]]

Attachment: [e-mail]

Comment 29

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified issue #51 (https://github.com/mozilla/shield-cloudstorage/issues/51) using the latest signed build and it is no longer reproducible.

From the QA point of view the add-on is ready to be released to end users.