Closed Bug 1462468 Opened 6 years ago Closed 6 years ago

Add a tracking protection option to the hamburger menu

Categories

(Firefox :: Menus, enhancement, P1)

Product:
Firefox
Component:
Menus
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 62
Tracking Flags:
Tracking Status
relnote-firefox --- 62+
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- unaffected
firefox62 + fixed

People

(Reporter: johannh, Assigned: johannh)

References

(Blocks 1 open bug)

Details

(Keywords: feature)

Attachments

(1 file)

Bug 1462468 - Add a tracking protection option to the hamburger menu.
59 bytes, text/x-review-board-request
mikedeboer
: review+
MarcoZ
: a11y-review+
Details 
We would like to promote tracking protection more prominently in the main (hamburger) menu, see https://mozilla.invisionapp.com/share/RSIY1B8GMC2#/screens/297824891
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
This is a WIP, still needs tests and some smaller details to figure out.
Comment on attachment 8979585 [details]
Bug 1462468 - Add a tracking protection option to the hamburger menu.

Marco, this commit adds a new toggle to the hamburger menu that allows the user to easily "flip" between tracking protection on and tracking protection off. The toggle heavily relies on visual cues, we have CSS showing its state through color and offset, hence I'm requesting a11y-review to make sure this button is still usable without the visual information.

It's still a regular toolbarbutton and I dynamically change the tooltiptext to reflect whether the button will enable or disable TP, so I hope it's not a big issue.

Thank you!
Attachment #8979585 - Flags: a11y-review?(mzehe)
Comment on attachment 8979585 [details]
Bug 1462468 - Add a tracking protection option to the hamburger menu.

a11y-r=me. These will probably all have to be revisited once we get to properly fixing keyboard accessibility for the customizable UI, but I don't know what that will look like yet. For now, this is fine with me.
Attachment #8979585 - Flags: a11y-review?(mzehe) → a11y-review+
One thing you could do, in the method where you set the enabled state and change the tooltip, do a setAttribute with the attribute aria-pressed, and its value true if enabled, false if disabled. That will turn this into a toggle button for assistive technologies and make it clear upfront what the state is.
Comment on attachment 8979585 [details]
Bug 1462468 - Add a tracking protection option to the hamburger menu.

https://reviewboard.mozilla.org/r/245724/#review252824

Fantastic, clean patch you got here ;-) It looks and works as expected. Thanks!

::: browser/themes/shared/customizableui/panelUI.inc.css:611
(Diff revision 2)
> +  position: relative;
> +  display: block;
> +  content: "";
> +  width: 10px;
> +  height: 10px;
> +  border-radius: 50%;

nit: Can you unify this border-radius statement with the one in the toggle button itself? (Settle for 10px or 50%.)
Attachment #8979585 - Flags: review?(mdeboer) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc39ccd992a4
Add a tracking protection option to the hamburger menu. r=mikedeboer
This bug has been noticed by the French press:
https://www.nextinpact.com/brief/firefox-63-expose-sa-gestion-plus-fine-des-parametres-de-vie-privee-4120.htm

We probably want to add this to the release notes.
relnote-firefox: --- → ?
Backed out changeset bc39ccd992a4 (bug 1462468) for Browser-chomre failures on browser/base/content/test/trackingUI/browser_trackingUI_appMenu.js

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=180569035&repo=autoland&lineNumber=2904

INFO - TEST-PASS | browser/base/content/test/trackingUI/browser_trackingUI_appMenu.js | The panel is closed to begin with. - "closed" == "closed" - 
[task 2018-05-28T14:17:43.322Z] 14:17:43     INFO - Buffered messages finished
[task 2018-05-28T14:17:43.322Z] 14:17:43     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/trackingUI/browser_trackingUI_appMenu.js | Test timed out - 
[task 2018-05-28T14:17:43.323Z] 14:17:43     INFO - GECKO(2516) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2018-05-28T14:17:43.323Z] 14:17:43     INFO - GECKO(2516) | MEMORY STAT | vsize 2238MB | residentFast 320MB | heapAllocated 107MB
[task 2018-05-28T14:17:43.324Z] 14:17:43     INFO - TEST-OK | browser/base/content/test/trackingUI/browser_trackingUI_appMenu.js | took 45253ms
[task 2018-05-28T14:17:43.324Z] 14:17:43     INFO - checking window state
[task 2018-05-28T14:17:43.328Z] 14:17:43     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-05-28T14:17:43.329Z] 14:17:43     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/trackingUI/browser_trackingUI_appMenu.js | Found a browser window after previous test timed out - 
[task 2018-05-28T14:17:43.330Z] 14:17:43     INFO - GECKO(2516) | must wait for focus
[task 2018-05-28T14:17:44.502Z] 14:17:44     INFO - GECKO(2516) | Completed ShutdownLeaks collections in process 2632
[task 2018-05-28T14:17:44.539Z] 14:17:44     INFO - GECKO(2516) | Completed ShutdownLeaks collections in process 2571
[task 2018-05-28T14:17:44.541Z] 14:17:44     INFO - GECKO(2516) | Completed ShutdownLeaks collections in process 2662
[task 2018-05-28T14:17:44.549Z] 14:17:44     INFO - GECKO(2516) | Completed ShutdownLeaks collections in process 2697
[task 2018-05-28T14:17:44.932Z] 14:17:44     INFO - GECKO(2516) | Completed ShutdownLeaks collections in process 2516
[task 2018-05-28T14:17:44.933Z] 14:17:44     INFO - TEST-START | Shutdown

Push that introduced these failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bc39ccd992a44e3c406afcb432c78d25c03046d6

Backout:
https://hg.mozilla.org/integration/autoland/rev/06788dddc02d16e56281a5e35091aad168db0d22
Flags: needinfo?(jhofmann)
Flags: needinfo?(jhofmann)
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f605d6efe347
Add a tracking protection option to the hamburger menu. r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/f605d6efe347
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
The button is hard to see in light mode. http://forums.mozillazine.org/viewtopic.php?f=23&t=3040368
Comment on attachment 8979585 [details]
Bug 1462468 - Add a tracking protection option to the hamburger menu.

https://reviewboard.mozilla.org/r/245724/#review253924

::: browser/locales/en-US/chrome/browser/browser.dtd:901
(Diff revision 4)
>  <!ENTITY getUserMedia.selectMicrophone.accesskey "M">
>  <!ENTITY getUserMedia.audioCapture.label "Audio from the tab will be shared.">
>  <!ENTITY getUserMedia.allWindowsShared.message "All visible windows on your screen will be shared.">
>  
>  <!ENTITY trackingProtection.title "Tracking Protection">
> +<!ENTITY trackingProtection.tooltip "Open Tracking Protection Preferences">

Are we OK with calling out "preferences" on all operating systems?
(In reply to Francesco Lodolo [:flod] from comment #22)
> Comment on attachment 8979585 [details]
> Bug 1462468 - Add a tracking protection option to the hamburger menu.
> 
> https://reviewboard.mozilla.org/r/245724/#review253924
> 
> ::: browser/locales/en-US/chrome/browser/browser.dtd:901
> (Diff revision 4)
> >  <!ENTITY getUserMedia.selectMicrophone.accesskey "M">
> >  <!ENTITY getUserMedia.audioCapture.label "Audio from the tab will be shared.">
> >  <!ENTITY getUserMedia.allWindowsShared.message "All visible windows on your screen will be shared.">
> >  
> >  <!ENTITY trackingProtection.title "Tracking Protection">
> > +<!ENTITY trackingProtection.tooltip "Open Tracking Protection Preferences">
> 
> Are we OK with calling out "preferences" on all operating systems?

I wondered that as well but we did it with the Sync prefs above and considering it's just a tooltip I went with it. If you feel strongly about it we might want to change both.
(In reply to Derek from comment #21)
> The button is hard to see in light mode.
> http://forums.mozillazine.org/viewtopic.php?f=23&t=3040368

Ah, that's unfortunate, thanks for letting me know. I'll file a follow-up bug. This should be easy to fix.
Depends on: 1465329
(In reply to Johann Hofmann [:johannh] from comment #23)
> I wondered that as well but we did it with the Sync prefs above and
> considering it's just a tooltip I went with it. If you feel strongly about
> it we might want to change both.

No strong opinion either, mostly double checking that it was expected. Let's stick to Preferences.
Depends on: 1465337
Hi,

QA would need details on what kind of QA involvement is expected for this. We need to understand how to test this and if this will be treated as a Bug work or we need to follow the full process (Test Plan, Sign Off etc.).

Could you please refer this (https://mana.mozilla.org/wiki/display/PI/PI+Request) and submit a PI request? We would need this submitted by June 4th, your earliest response would be appreciated!

Thank you!
Flags: needinfo?(jhofmann)
Hi Tania,

since I'm on PTO next week I would like to defer the PI Request to Julie, who is probably also more on top of this stuff. :)

Julie, would you mind taking this? I'm happy to coordinate if you have any questions. Just ping me!

> QA would need details on what kind of QA involvement is expected for this. We need to understand how to test this and if this will be treated as a Bug work or we need to follow the full process (Test Plan, Sign Off etc.).

I would say the former. Bug 1461743 is tracking individual improvements to the tracking protection UI/UX, there's no greater "feature" and this isn't planned to be behind a pref right now.

Thanks!
Flags: needinfo?(jhofmann) → needinfo?(jmccracken)
(In reply to Sylvestre Ledru [:sylvestre] from comment #12)
> This bug has been noticed by the French press:
> https://www.nextinpact.com/brief/firefox-63-expose-sa-gestion-plus-fine-des-
> parametres-de-vie-privee-4120.htm
> 
> We probably want to add this to the release notes.

Added to Nightly 62 release notes with this wording:
Added a button to the hamburger menu to toggle Tracking Protection on and off
(In reply to Johann Hofmann [:johannh] - Away until June 10 from comment #27)
> Hi Tania,
> 
> since I'm on PTO next week I would like to defer the PI Request to Julie,
> who is probably also more on top of this stuff. :)
> 
> Julie, would you mind taking this? I'm happy to coordinate if you have any
> questions. Just ping me!
> 
> > QA would need details on what kind of QA involvement is expected for this. We need to understand how to test this and if this will be treated as a Bug work or we need to follow the full process (Test Plan, Sign Off etc.).
> 
> I would say the former. Bug 1461743 is tracking individual improvements to
> the tracking protection UI/UX, there's no greater "feature" and this isn't
> planned to be behind a pref right now.
> 
> Thanks!

This is to confirm that the PI-Request was submitted to test this.
No longer depends on: 1466844
Depends on: 1468311
Depends on: 1468312
Depends on: 1468313
Depends on: 1468314
Depends on: 1468316
Depends on: 1468318
Depends on: 1468323
Depends on: 1468249
No longer depends on: 1468323
No longer depends on: 1468312
No longer depends on: 1468314
Flags: needinfo?(jmccracken)
Depends on: 1478545
Depends on: 1483364
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: