1321302 - Switch in-content checkboxes back to pseudoelements

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

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

Checkboxes (and radio inputs) haven't traditionally been very stylable in Gecko, even with
-moz-appearance: none;. That was fixed in bug 418833, but those patches will not be uplifted
in time for 52.

In bug 1309316, I had counted on getting bug 418833 landed in time to make 52, and so had
written some CSS to switch us to using traditional checkboxes (with the new styling abilities)
rather than the old mechanism, which was a hacky pseudoelement solution. Since bug 418833
didn't make the uplift, we have to go back to the old mechanism.

That's what this bug is about.

Comment 1

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

This depends on a backout of bug 1317795 on aurora as well.

Comment 2

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

Attachment: Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

Checkboxes (and radio inputs) haven't traditionally been very stylable in Gecko, even with
-moz-appearance: none;. That was fixed in bug 418833, but those patches will not be uplifted
in time for 52.

In bug 1309316, I had counted on getting bug 418833 landed in time to make 52, and so had
written some CSS to switch us to using traditional checkboxes (with the new styling abilities)
rather than the old mechanism, which was a hacky pseudoelement solution. Since bug 418833
didn't make the uplift, we have to go back to the old mechanism. That's what this patch
does.

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

Comment 3

[:Gijs (he/him)]

Comment on attachment 8815765 [details]
Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

https://reviewboard.mozilla.org/r/96574/#review96784

rs=me based on the patch from bug 1309316. Do we also need to (can we?) remove the `.checkbox-with-label` rules at the bottom of aboutTabCrashed.css ?

Comment 4

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

Comment on attachment 8815765 [details]
Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

https://reviewboard.mozilla.org/r/96574/#review96784

Thanks! Yeah - this patch will land after a backout of bug 1317795, which will remove those rules.

Comment 5

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

Comment on attachment 8815765 [details]
Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

Approval Request Comment
[Feature/Bug causing the regression]:

Bug 1309316

[User impact if declined]:

In-content checkboxes (seen in, for example, about:networking, about:tabcrashed, about:debugging, about:neterror) will look "strange", and not according to spec.

[Is this code covered by automated tests?]:

Unfortunately, no - but this is a partial backout of bug 1309316's changes which puts us back to a known good state that we had been shipping for quite a while.

[Has the fix been verified in Nightly?]:

This patch will not land on Nightly - it's for Aurora-only. The patch is a partial backout of bug 1309316, which puts us back to a known good state that Aurora (and Beta, and Release) has shipped with for months.

[Needs manual test from QE? If yes, steps to reproduce]: 

It would be good to get some additional eyes on this for manual verification.

STR:

1) Visit about:debugging
2) Ensure that the checkbox before the "Enable add-on debugging" label looks correct. See PNG attachment that demonstrates what "correct" and "incorrect" looks like.
3) Ensure that the checkbox can be clicked, and that it updates its visual state.

[List of other uplifts needed for the feature/fix]:

We'll need to backout bug 1317795 from Aurora.

[Is the change risky?]:

No.

[Why is the change risky/not risky?]:

We're putting Aurora back into a known good state.

[String changes made/needed]:

None.

Comment 6

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

Attachment: Good vs bad checkboxes to aid verification

Comment 7

[Julien Cristau [:jcristau]]

Comment on attachment 8815765 [details]
Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

change in-content checkboxes back to a working state for 52

Comment 8

[Carsten Book [:Tomcat]]

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

Comment 9

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

This only needed to land on Aurora, so we're done here.

Comment 10

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

Comment on attachment 8815765 [details]
Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

Approval Request Comment
[Feature/Bug causing the regression]:

See bug 1352406

[User impact if declined]:

Users will see unstyled, not-to-UX-spec in a number of our in-content pages (about:debugging, about:tabcrashed, about:networking, about:neterror and about:performance).

[Is this code covered by automated tests?]:

This is a styling change, so not really.

[Has the fix been verified in Nightly?]:

No. This is skipping Nightly and going straight to Beta. lizzard knows the specifics - see bug 1352406 for details.

[Needs manual test from QE? If yes, steps to reproduce]:

Couldn't hurt. Go to the about: pages I listed above, and look at the checkboxes. This will help with visual verification: https://bug1321302.bmoattachments.org/attachment.cgi?id=8815778 (top is good, bottom is bad).

[List of other uplifts needed for the feature/fix]:

See bug 1352406. It is required that a number of backouts land before this lands. Please coordinate with mconley before attempting a landing.

[Is the change risky?]:

Not this patch, no.

[Why is the change risky/not risky?]:

Simple, well understood styling change that we shipped in 52 already.

[String changes made/needed]:

None.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8815765 [details]
Bug 1321302 - Undo changes from bug 1309316 that switches in-content checkboxes to real ones instead of pseudoelements.

As discussed in bug 1309316.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cfc4e84b9679