Switch in-content checkboxes back to pseudoelements

RESOLVED FIXED

Status

()

Firefox
Theme
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

52 Branch
Points:
---

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 fixed, firefox53 fixed, firefox54 unaffected, firefox55 unaffected)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

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.
Assignee: nobody → mconley
This depends on a backout of bug 1317795 on aurora as well.
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
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 ?
Attachment #8815765 - Flags: review?(gijskruitbosch+bugs) → review+
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 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.
Attachment #8815765 - Flags: approval-mozilla-aurora?
Created attachment 8815778 [details]
Good vs bad checkboxes to aid verification
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
Attachment #8815765 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 8

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/f6f2e2b9fa87
status-firefox52: affected → fixed
This only needed to land on Aurora, so we're done here.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
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.
Attachment #8815765 - Flags: approval-mozilla-beta?
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.
Attachment #8815765 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox53: unaffected → affected
status-firefox54: --- → unaffected
status-firefox55: --- → unaffected
You need to log in before you can comment on or make changes to this bug.