Closed Bug 1649301 Opened 11 months ago Closed 9 months ago

Add privacy screen in multistage about:welcome onboarding Fx 80

Categories

(Firefox :: Messaging System, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
81 Branch
Iteration:
81.1 - July 27 - Aug 09
Tracking Status
firefox79 --- unaffected
firefox80 + fixed
firefox81 --- verified

People

(Reporter: pdahiya, Assigned: emcminn)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

Jim, is this something we have enough support for to land in 80?

Flags: needinfo?(jimthomas)

Updating bug with design specs and assets

https://www.figma.com/proto/KqQZwwLhft9cUIcBtMVmbz/Desktop-First-Run?node-id=1495%3A9&viewport=-4324%2C165%2C1&scaling=contain

Updated copy:
“Firefox automatically blocks trackers and malware, and keeps companies from secretly following you around. When you see the shield while browsing, Firefox is protecting you.

Attached video Onbboarding 1x.mp4
Attached video Onbboarding@2x.mp4
Attached image firefox_protections.svg

this is targeted for Fx80

Assignee: nobody → emcminn
Iteration: --- → 80.2 - July 13 - July 26
Flags: needinfo?(jimthomas)
Priority: -- → P1

Hi Eric, would it be possible to get a .png version of the privacy animation? The implementation would be much easier :)

Flags: needinfo?(epang)

Hi Eric

Couple of things that came out of review feedback

a) Can we get video in both mp4/webm and apng format to compare file sizes. My understanding video is going to be same in light and dark theme, is that correct? If transparency is required than we need apng format.

Either way file size should be prioritized, if mp4/webm compresses better than apng, then it might be worth dropping transparency requirement. There is a chance using animation is going to cause installer size regressions (like it happened with the pinned tabs ~0.5%+) .

b) need to support RTL: this means a separate asset (A mirrored asset wouldn't be a correct representation as the ETP icon looks the same as in LTR)

Thanks!

Attached file APNGs.zip

Hi Emily, I've attached two APNGs at 1x and 2x. The file size seems to be larger than the mp4s so I'm not sure what the impact will be. Let me know! The transparency is nice though since we don't need to worry about 2 versions for light and dark themes!

Flags: needinfo?(epang) → needinfo?(emcminn)

(In reply to Punam Dahiya [:pdahiya] from comment #9)

Hi Eric

Couple of things that came out of review feedback

a) Can we get video in both mp4/webm and apng format to compare file sizes. My understanding video is going to be same in light and dark theme, is that correct? If transparency is required than we need apng format.

I've attached apngs to try out. Let me know how they work out. If we need to use the mp4 files we'll need to create a dark theme version as well. I've also tried to compress the mp4s a little more and also exported webm (which seem to be even smaller) - I will attach to the bug

Either way file size should be prioritized, if mp4/webm compresses better than apng, then it might be worth dropping transparency requirement. There is a chance using animation is going to cause installer size regressions (like it happened with the pinned tabs ~0.5%+) .

b) need to support RTL: this means a separate asset (A mirrored asset wouldn't be a correct representation as the ETP icon looks the same as in LTR)

When we spoke yesterday it sounds like we don't need to worry about this for the test. When needed I can create these assets :)

Thanks!

Attached file video assets.zip

Adding some assets to help figure out which will work best. I've tried to compress the mp4s a little more. I've also exported webm files that seem to be smaller in file size.

Thanks Eric! Working on it now.

Flags: needinfo?(emcminn)

emcminn reached out because of an issue loading the video element in the about:welcome page. Specifically, this error being thrown in the console:

Security Error: Content at about:welcome may not load or link to resource://activity-stream/data/content/assets/privacy-onboarding.mp4.

It looks like this is happening via this line: https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/caps/nsScriptSecurityManager.cpp#950-951

because we're failing this flag check: https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/caps/nsScriptSecurityManager.cpp#886

It looks like for things like CSS style sheets, the nsILoadInfo::ALLOW_CHROME flag is passed to through the channel: https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/layout/style/Loader.cpp#1187-1189. This is what allows the resource://activity-stream/aboutwelcome/aboutwelcome.css stylesheet to be loaded successfully from a resource:// URI.

But video elements, we do differently. Here's where the load is initiated: https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/dom/html/HTMLMediaElement.cpp#1729-1772

It doesn't look to me like we ever give the opportunity for the load to have nsILoadInfo::ALLOW_CHROME.

So I think that's the problem. Not sure what the right solution is though - this is something that we should probably needinfo someone deeply familiar with DOM Security about. I'm going to needinfo ckerschb to see if he has a sense of the right way forward here.

Flags: needinfo?(ckerschb)

[Tracking Requested - why for this release]: this is required for onboarding experiment in 80 (v3) where we are investigating privacy/security messaging during first-run

Iteration: 80.2 - July 13 - July 26 → 81.1 - July 27 - Aug 09

(In reply to Mike Conley (:mconley) (:⚙️) from comment #14)

So I think that's the problem. Not sure what the right solution is though - this is something that we should probably needinfo someone deeply familiar with DOM Security about. I'm going to needinfo ckerschb to see if he has a sense of the right way forward here.

Yes, not passing the nsILoadInfo::ALLOW_CHROME flag is exactly the reason the .mp4 can not be accessed. I personally would consider that mp4 to be similar to an image load and hence we could simply always pass the nsILoadInfo::SEC_ALLOW_CHROME flag. I guess we just never had the use case to expose a packaged .mp4 before.

To make sure I would like to double check with :bholley to see if we should just do that, or if it would be preferred to make the passing of the flag conditionally do any additional parameter.

Bobby?

Flags: needinfo?(ckerschb) → needinfo?(bholley)

Treating media elements like images in this case seems like the right thing to me.

Flags: needinfo?(bholley)
Depends on: 1655765

Hi Christoph, could you verify the fix in the blocking bug? If that's correct then I should be able to rebase this one on top of it to catch the fix :)

Flags: needinfo?(ckerschb)

(In reply to Emily McMinn :emcminn from comment #18)

Hi Christoph, could you verify the fix in the blocking bug? If that's correct then I should be able to rebase this one on top of it to catch the fix :)

Yep, already r+ed the fix.

Flags: needinfo?(ckerschb)
Attachment #9163557 - Attachment description: Bug 1649301 - Basic privacy page → Bug 1649301 - Privacy page, with optimized media files

Hi Eric, would you mind taking a look at the attached patch? (I can get screenshots instead if you'd prefer.) We had to make some changes to the dimensions of the video as compared to the spec, and we wanted to get your opinion on the bottom transparency too. WebM is the smallest format, but webm/mp4 doesn't support the transparency.

Flags: needinfo?(epang)
Pushed by emcminn@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/07812e460d4f
Privacy page, with optimized media files r=pdahiya
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch

I have verified that the privacy screen is correctly displayed in the "about:welcome" page by setting the value of the browser.aboutwelcome.overrideContet pref to {"id": "multi-stage-welcome-importable-theme", "template": "multistage", "screens": [{"id":"AW_GET_STARTED", "order": 0, "content": {"zap": true, "title": {"string_id": "onboarding-multistage-welcome-header"}, "subtitle": {"string_id": "onboarding-multistage-welcome-subtitle"}, "primary_button": {"label": {"string_id": "onboarding-multistage-welcome-primary-button-label"}, "action": {"navigate": true}},"secondary_button": {"text": {"string_id": "onboarding-multistage-welcome-secondary-button-text"}, "label": {"string_id": "onboarding-multistage-welcome-secondary-button-label"}, "position": "top", "action": {"type": "OPEN_URL", "addFlowParams": true, "data": {"args": "https://accounts.firefox.com/?service=sync&action=email&context=fx_desktop_v3&entrypoint=activity-stream-firstrun&style=trailhead", "where": "current"}}}}}, {"id": "AW_IMPORT_SETTINGS", "order": 1, "content": { "zap": true, "disclaimer": {"string_id": "onboarding-import-sites-disclaimer"}, "title": {"string_id": "onboarding-multistage-import-header"}, "subtitle": {"string_id": "onboarding-multistage-import-subtitle"}, "tiles": {"type": "topsites"}, "primary_button": { "label": {"string_id": "onboarding-multistage-import-primary-button-label"}, "action": {"type": "SHOW_MIGRATION_WIZARD", "navigate": true}}, "secondary_button": { "label": {"string_id": "onboarding-multistage-import-secondary-button-label"}, "action": {"navigate": true}}}}, {"id": "AW_CHOOSE_THEME", "order": 2, "content": { "zap": true, "title": {"string_id": "onboarding-multistage-theme-header"}, "subtitle": {"string_id": "onboarding-multistage-theme-subtitle"}, "tiles": {"type": "theme", "action": {"theme": "<event>"}, "data": [{"theme": "light", "label": {"string_id": "onboarding-multistage-theme-label-light"}}, {"theme": "dark", "label": {"string_id": "onboarding-multistage-theme-label-dark"}}]}, "primary_button": {"label": {"string_id": "onboarding-multistage-theme-primary-button-label"}, "action": { "navigate": true}}, "secondary_button": { "label": {"string_id": "onboarding-multistage-theme-secondary-button-label"}, "action": {"theme": "default", "navigate": true}}}}, { "id": "AW_PRIVACY", "order": 3, "content": { "title": "Automatic Privacy", "subtitle": "Firefox automatically blocks trackers and malware, and keeps companies from secretly following you around. When you see the shield while browsing, Firefox is protecting you.", "tiles": {"type": "video", "media_type": "privacy", "source": { "default" : "resource://activity-stream/data/content/assets/privacy-onboarding.webm", "dark" : "resource://activity-stream/data/content/assets/privacy-onboarding-dark.webm"}}, "primary_button": { "label": "Start Browsing", "action": {"navigate": true}}}}]}.

I've verified this by using Firefox Nightly 81.0a1 (Build ID: 20200806033456) on Windows 10, macOS 10.15, and Ubuntu Linux 18.04.

Status: RESOLVED → VERIFIED

Is this ready to uplift?

Flags: needinfo?(emcminn)

clearing ni since we discussed on slack.

Flags: needinfo?(epang)

Hi Julien, the patch is ready, we were just handling the installer size regression. Both the opened bugs have a response now, so I think we should be good to go. We'll be expecting a 0.34% installer size regression in 80 after the uplift, which we plan to fix in a followup.
Thanks!

Flags: needinfo?(emcminn)

Comment on attachment 9163557 [details]
Bug 1649301 - Privacy page, with optimized media files

Beta/Release Uplift Approval Request

  • User impact if declined: This privacy screen is needed for the V3 about:welcome experiment; it will be behind a pref and will only be seen by users enrolled in the experiment.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This screen is currently experiment-only, and will only be seen by users enrolled in the correct branch of the V3 about:welcome experiment.
  • String changes made/needed: none
Attachment #9163557 - Flags: approval-mozilla-beta?

Comment on attachment 9163557 [details]
Bug 1649301 - Privacy page, with optimized media files

approved for 80.0b7

Attachment #9163557 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.