[Tracking Requested - why for this release]: Regression STR: 1. Open this website page - https://forum.doom9.org/showthread.php?t=166689 2. See that checkbox is rendered differently compared to the other checkboxes in about:preferences

This is fallout from bug 1309316. I hope to fix this with bug 418833 (and hopefully get the uplift).

Thank you very much for finding the regression range.

(ah, cool, I was gonna get around to looking into why the checkbox in about:debugging looks different... seems like this is why!)

Please don't forget to fix the switch in about:privatebrowsing

Adding also to it about:networking and about:performance

The regression for about:privatebrowsing can (and should) be fixed separately, so I've re-opened bug 1317804.

Note that this work relies on the patches in bug 418833 to look fully appropriate. ntim, are you the right reviewer for in-content changes like this?

Comment on attachment 8812499 [details] Bug 1317795 - Fix broken alignment of in-content UI checkboxes. https://reviewboard.mozilla.org/r/94224/#review94372 I think Gijs or jaws would be more appropriate to review this kind of changes, but here are a few comments anyway. ::: devtools/client/aboutdebugging/aboutdebugging.css:185 (Diff revision 1) > flex: 1; > } > > .addons-debugging-label { > display: inline-block; > - margin: 0 5px 5px 0; > + margin-right: 5px; nit: Use RTL aware prop (margin-inline-end). ::: toolkit/components/aboutperformance/content/aboutPerformance.xhtml:148 (Diff revision 1) > + .options > input[type="checkbox"]:not(:first-child) { > + margin-left: 10px; > + } hmm, this is an ugly hack. I'd rather have two containers as children containing the 2 checkboxes. ::: toolkit/themes/shared/in-content/common.inc.css:557 (Diff revision 1) > > xul|richlistitem > xul|*.checkbox-check { > margin: 3px 6px; > } > > +html|*.checkbox-container-with-text { This is a class we'll probably be able to use with radios as well, so maybe rename it as toggle-with-label ? ::: toolkit/themes/shared/in-content/common.inc.css:565 (Diff revision 1) > + > +html|*.checkbox-container-with-text > label, > +html|*.checkbox-container-with-text > span, > +html|*.checkbox-container-with-text > a { > + margin-top: auto; > + margin-bottom: auto; Instead of specifying margins, does using one of those flex properties work (you might need to apply them on the parent) ? I'm thinking of align-self/content

(In reply to Mike Conley (:mconley) from comment #11) > Note that this work relies on the patches in bug 418833 to look fully > appropriate. > > ntim, are you the right reviewer for in-content changes like this? I think :Gijs or :jaws would be more appropriate to review this since they are browser peer. Plus I'm not accepting review requests right now :)

Drive-by: I haven't tried this patch locally but it doesn't seem to touch the aboutNetError/CertError page, which also has an unstyled checkbox now. Please make sure that that one is also fixed :)

Good call, thank you!

Comment on attachment 8812499 [details] Bug 1317795 - Fix broken alignment of in-content UI checkboxes. https://reviewboard.mozilla.org/r/94224/#review94372 > This is a class we'll probably be able to use with radios as well, so maybe rename it as toggle-with-label ? I chose "toggle-container-with-text", since we're dealing with non-label nodes as well (<span>, <a>, at least)

Comment on attachment 8812499 [details] Bug 1317795 - Fix broken alignment of in-content UI checkboxes. https://reviewboard.mozilla.org/r/94224/#review94372 > Instead of specifying margins, does using one of those flex properties work (you might need to apply them on the parent) ? I'm thinking of align-self/content Ah, align-items on the container was the rule I wanted. Thank you!

Comment on attachment 8812499 [details] Bug 1317795 - Fix broken alignment of in-content UI checkboxes. https://reviewboard.mozilla.org/r/94224/#review94710 ::: devtools/client/aboutdebugging/aboutdebugging.css:185 (Diff revision 2) > flex: 1; > } > > .addons-debugging-label { > display: inline-block; > - margin: 0 5px 5px 0; > + margin-inline-end: 5px; Please use 1ch here instead of 5px. ::: toolkit/components/aboutperformance/content/aboutPerformance.xhtml:152 (Diff revision 2) > + } > + .options > .toggle-container-with-text { > + display: inline-flex; > + } > + .options > .toggle-container-with-text:not(:first-child) { > + margin-left: 10px; margin-inline-start: 2ch;

Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e241e4cec47a Fix broken alignment of in-content UI checkboxes. r=jaws

Tracking 53- since this is now resolved fixed.

Mike, can you please request Aurora approval on this when you get a chance?

Comment on attachment 8812499 [details] Bug 1317795 - Fix broken alignment of in-content UI checkboxes. Approval Request Comment [Feature/regressing bug #]: Bug 1309316, bug 418833 [User impact if declined]: Checkboxes and their labels will be slightly misaligned. [Describe test coverage new/current, TreeHerder]: All manual, as this is a styling thing. [Risks and why]: Very low risk. This just adjusts the majority of the in-content checkboxes to use the new unstyled checkbox properly. [String/UUID change made/needed]: None.

tracking for 52 as new regression

Comment on attachment 8812499 [details] Bug 1317795 - Fix broken alignment of in-content UI checkboxes. styling fix for aurora52

Note that it looks like bug 418833 is not going to uplift. This bugfix kinda depended on that, so this might get backed out in short order. I'm going to work out a good solution with UX before making a move.

Here is my proposal for fixing this on Aurora 52: 1) Back out this patch from Aurora, since bug 418833 will not uplift. 2) Undo the changes from bug 1309316 that switch us to using the standard checkbox, and put us back to the pseudoelement checkbox 3) Use some margin and text-indent hackery to make text wrapping work acceptably well on about:tabcrashed. This will probably have to do until 53. I'll need to get UX sign-off on this. The first two steps can be done immediately. Step 1 is trivial. Step 2, I will have a patch prepared for today.

I've filed bug 1321302 for Part 2, and bug 1321306 for Part 3.