1317804 - Private browsing about page has non-functioning checkbox appended to 'Tracking Protection' label

Status: VERIFIED FIXED

(Firefox :: Private Browsing, defect)

Description

[Jim Mathies [:jimm]]

Attachment: trackingcheck.jpg

This appears to be an oversized web form checkbox. Clicking on it doesn't change the state though.

Comment 1

[Johann Hofmann [:johannh]]

Huh, first of all this checkbox should not be shown. I can reproduce on Nightly, I think that regressed with bug 1309316. Should be easy to fix.

However, I can't reproduce

> Clicking on it doesn't change the state though.

and I'm also confused that you don't seem to have the green/gray "toggle" next to the checkbox. Can you check for errors in the page console and the browser console?

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: screenshot.png

Duplicate of bug #1317795 ?

BTW I'm able to change status of Tracking Protection using both checkboxes in Private Mode (Windows 7 [64bit] + Nightly [64it]).

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

> Duplicate of bug #1317795 ?

Looks like it, thanks!

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Actually, I think this is kinda separate from bug 1317795. The checkbox is _never_ supposed to be visible.

I think the situation jimm was in was occurs when privacy.trackingprotection.enabled is set to true, which enables global tracking protection. I am easily able to reproduce the issue with that pref set.

I think I can fix this separately from the checkbox restyling that's going to occur elsewhere once bug 418833 lands.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1317804 - Hide the checkbox toggle in about:privatebrowsing.

This was originally taking advantage of the fact that we were hiding
HTML checkboxes by default via common.inc.css. Since bug 1309316
landed, we don't hide those checkboxes by default anymore.

So now we special-case about:privatebrowsing to hide the checkbox.

Review commit: https://reviewboard.mozilla.org/r/94216/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94216/

Comment 6

[Johann Hofmann [:johannh]]

(In reply to Mike Conley (:mconley) from comment #4)
> I think the situation jimm was in was occurs when
> privacy.trackingprotection.enabled is set to true, which enables global
> tracking protection. I am easily able to reproduce the issue with that pref
> set.
> 
> I think I can fix this separately from the checkbox restyling that's going
> to occur elsewhere once bug 418833 lands.

Ah, good find. I don't think that this needs any more fixing except hiding the checkbox though :)

Comment 7

[Johann Hofmann [:johannh]]

Comment on attachment 8812490 [details]
Bug 1317804 - Hide the checkbox toggle in about:privatebrowsing.

https://reviewboard.mozilla.org/r/94216/#review94402

Thanks, this seems to work fine!

::: browser/themes/shared/privatebrowsing/aboutPrivateBrowsing.css:142
(Diff revision 1)
>  
> +.toggle-input {
> +  opacity: 0;
> +  width: 0;
> +  pointer-events: none;
> +  position: absolute;

A minor note:

As a relative newcomer to this code, I had to spend some time reading through blame and old bugs to find out why exactly this is hidden like that and not just using "display:none" (apparently to preserve keyboard focus).

Can you add a really short comment explaining that? The recent changes will make it much harder to get the blame info in the future.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8812490 [details]
Bug 1317804 - Hide the checkbox toggle in about:privatebrowsing.

https://reviewboard.mozilla.org/r/94216/#review94402

> A minor note:
> 
> As a relative newcomer to this code, I had to spend some time reading through blame and old bugs to find out why exactly this is hidden like that and not just using "display:none" (apparently to preserve keyboard focus).
> 
> Can you add a really short comment explaining that? The recent changes will make it much harder to get the blame info in the future.

Thanks for the review! That's a really good call - thanks johannh! (And welcome back! :D)

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8812490 [details]
Bug 1317804 - Hide the checkbox toggle in about:privatebrowsing.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94216/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b5e7c9b5c8c
Hide the checkbox toggle in about:privatebrowsing. r=johannh

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9b5e7c9b5c8c

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8812490 [details]
Bug 1317804 - Hide the checkbox toggle in about:privatebrowsing.

Approval Request Comment
[Feature/regressing bug #]:

Bug 1309316.

[User impact if declined]:

A non-native checkbox (sometimes non-functional in a non-default case) will be displayed in about:privatebrowsing.

[Describe test coverage new/current, TreeHerder]:

Testing was manual, but this is a page styling thing, so that's about par for the course here.

[Risks and why]: 

Very low risk. We're just hiding the checkbox.

[String/UUID change made/needed]:

None.

Comment 14

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8812490 [details]
Bug 1317804 - Hide the checkbox toggle in about:privatebrowsing.

styling regression fix in about:privatebrowsing, take in aurora52

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fbb6f22204ac

Comment 16

[Sajedul Islam]

I have reproduced this bug with Nightly 53.0a1 (2016-11-15) on Windows 8.1 , 64 Bit ! 

Build   ID    20161115030213
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

This bug's fix is Verified with latest Aurora and latest Nightly !

Build   ID    20161206004003
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0

Build   ID    20161205030204
User Agent    Mozilla/5.0 (Windows NT 6.3; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20161207]

Comment 17

[Johann Hofmann [:johannh]]

Thanks!

Comment 18

[u576612]

[bugday-20170104] Bug verified

Comment 19

[Surentharan]

[bugday-20170104] Bug verified

Comment 20

[66makilan66]

[bugday-1317804] bug verified
sorry about previous comment