Add a tracking protection option to the hamburger menu

RESOLVED FIXED in Firefox 62

Status

()

P1
normal
RESOLVED FIXED
10 months ago
7 months ago

People

(Reporter: johannh, Assigned: johannh)

Tracking

(Blocks: 1 bug, {feature})

unspecified
Firefox 62
feature
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 62+, firefox-esr52 unaffected, firefox-esr60 unaffected, firefox60 unaffected, firefox61 unaffected, firefox62+ fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

10 months ago
We would like to promote tracking protection more prominently in the main (hamburger) menu, see https://mozilla.invisionapp.com/share/RSIY1B8GMC2#/screens/297824891
(Assignee)

Updated

10 months ago
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
(Assignee)

Comment 3

10 months ago
This is a WIP, still needs tests and some smaller details to figure out.
Comment hidden (mozreview-request)
(Assignee)

Comment 6

10 months ago
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 9

10 months ago
mozreview-review
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+
Comment hidden (mozreview-request)

Comment 11

10 months ago
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: --- → ?

Comment 13

10 months ago
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)

Updated

10 months ago
Keywords: feature
(Assignee)

Updated

10 months ago
Flags: needinfo?(jhofmann)
Comment hidden (mozreview-request)

Comment 19

10 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f605d6efe347
Add a tracking protection option to the hamburger menu. r=mikedeboer

Comment 20

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f605d6efe347
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62

Comment 21

10 months ago
The button is hard to see in light mode. http://forums.mozillazine.org/viewtopic.php?f=23&t=3040368

Comment 22

10 months ago
mozreview-review
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?
(Assignee)

Comment 23

10 months ago
(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.
(Assignee)

Comment 24

10 months ago
(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.
(Assignee)

Updated

10 months ago
Depends on: 1465329

Updated

10 months ago
status-firefox60: --- → unaffected
status-firefox61: --- → unaffected
status-firefox-esr52: --- → unaffected
status-firefox-esr60: --- → unaffected
tracking-firefox62: --- → +
(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.
(Assignee)

Updated

10 months ago
Depends on: 1465337

Updated

10 months ago
Depends on: 1465444
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)
(Assignee)

Comment 27

10 months ago
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.
(Assignee)

Updated

10 months ago
No longer depends on: 1466844
(Assignee)

Updated

10 months ago
Depends on: 1468249
(Assignee)

Updated

10 months ago
No longer depends on: 1468323
(Assignee)

Updated

9 months ago
No longer depends on: 1468312
(Assignee)

Updated

9 months ago
No longer depends on: 1468314
relnote-firefox: ? → 62+
(Assignee)

Updated

8 months ago
Flags: needinfo?(jmccracken)
Depends on: 1478545
(Assignee)

Updated

7 months ago
Depends on: 1483364
You need to log in before you can comment on or make changes to this bug.