Closed
Bug 1462468
Opened 7 years ago
Closed 6 years ago
Add a tracking protection option to the hamburger menu
Categories
(Firefox :: Menus, enhancement, P1)
Firefox
Menus
Tracking
()
RESOLVED
FIXED
Firefox 62
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)
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 | ||
Updated•7 years ago
|
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
Assignee | ||
Comment 3•7 years ago
|
||
This is a WIP, still needs tests and some smaller details to figure out.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years 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 7•7 years ago
|
||
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+
Comment 8•7 years ago
|
||
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•6 years 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•6 years 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
Comment 12•6 years ago
|
||
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•6 years 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)
Assignee | ||
Comment 14•6 years ago
|
||
Assignee | ||
Comment 15•6 years ago
|
||
Assignee | ||
Comment 16•6 years ago
|
||
Assignee | ||
Comment 17•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jhofmann)
Comment hidden (mozreview-request) |
Comment 19•6 years 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•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Comment 21•6 years ago
|
||
The button is hard to see in light mode. http://forums.mozillazine.org/viewtopic.php?f=23&t=3040368
Comment 22•6 years 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•6 years 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•6 years 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.
Updated•6 years ago
|
status-firefox60:
--- → unaffected
status-firefox61:
--- → unaffected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
tracking-firefox62:
--- → +
Comment 25•6 years ago
|
||
(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.
Comment 26•6 years ago
|
||
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•6 years 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)
Comment 28•6 years ago
|
||
(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
Comment 29•6 years ago
|
||
(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.
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jmccracken)
You need to log in
before you can comment on or make changes to this bug.
Description
•