Closed Bug 1483527 Opened 2 years ago Closed 2 years ago
Change fieldset's default padding to match other browsers
46 bytes, text/x-phabricator-request
|Details | Review|
This looks trivial - I can take it.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10edee9dabfb0eefe966257d022461d5bbc2f4fc (I anticipate we'll need to update some tests' expectations)
Historical note -- Firefox's current padding (0.35em on top, 0.625em on sides, .75em on bottom) dates back 15 years to https://bugzilla.mozilla.org/show_bug.cgi?id=234740 where we made a tweak to match IE on WinXP. Here's that patch: attachment 143216 [details] [diff] [review] / https://searchfox.org/mozilla-central/commit/491c58de08db4dfd96753bc6aff67392d66f5bcc It's stayed the same since then, aside from the switch to using logical properties for symmetry under a vertical writing mode.
Depends on: 234740
Historical note for Blink/Edge/WebKit: - Obviously Blink got its behavior via forking WebKit, and Edge presumably switched for compat/interop reasons. - So why did WebKit have this particular behavior (diverging from Firefox/Edge)? I traced the blame on GitHub and it looks like they *originally* had "padding: 0.75em 0.625em;" exactly like Firefox did before we adopted our current behavior (to mirror IE) in bug 234740. WebKit changed from that 2-value syntax to "padding: 0.35em 0.75em 0.625em" **with the last two values swapped, as compared to Firefox!**, as an apparently-unrelated ridealong change in an otherwise unrelated-to-fieldset commit from Hyatt in 2005. The commit message was: > Meant to land this ages ago. Make radio buttons work dynamically > when name and type change (make them pick up the correct new group). https://github.com/WebKit/webkit/commit/fc93977b351a9a4caa8b05e15185fabbdc8f2be3 So, I suspect this was a typo in WebKit, where they intended to come into interoperability with Firefox/IE but accidentally swapped the last two values in their 3-value padding syntax. It's possible that this was included in that commit entirely by accident (i.e. it could've been a local tweak that got merged in at the last minute, perhaps). Hard to say, because there's no reference to a bug number in the commit.
Comment on attachment 9001324 [details] Bug 1483527: Change fieldset block-end & inline-axis padding to match other browsers. Emilio Cobos Álvarez (:emilio) has approved the revision.
Attachment #9001324 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/3d98e379dc38 Change fieldset block-end & inline-axis padding to match other browsers. r=emilio
Backed out 1 changesets (bug 1483527) for fieldset reftest failures push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3d98e379dc389fce823507040d3ce2a77970fb35&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&fromchange=45e540e5e0fc2bfca60fe015a546fdb6f2f01318&selectedJob=194341743&filter-searchStr=Windows+10+x64+QuantumRender+opt+Reftests+with+e10s+test-windows10-64-qr%2Fopt-reftest-e10s-2+R-e10s%28R2%29 log: https://treeherder.mozilla.org/logviewer.html#?job_id=194341743&repo=autoland backout: https://hg.mozilla.org/integration/autoland/rev/dbf1387f8b3603fb0f57898724853c35047c63a0
This was an "unexpected pass", for a fieldset test that is annotated as fuzzy under webrender (with the fuzzy pixels being a different shade around the edges of the fieldset). Apparently it's slightly "less fuzzy" (fewer pixels are fuzzy), probably due to a different number of pixels being present in the border, due to the padding change. The annotation is: > fuzzy-if(webrender,17-19,1221-2452) == sticky-legend-1.html sticky-legend-1-ref.html And the "unexpected pass" had: max difference: 17, number of differing pixels: 1217 Notably 1217 is less than the annotation's lower-bound of 1221. I'll include a tweak to reduce 1221 to 1217 in the reftest annotation when I re-push.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/2b5fc64beafb Change fieldset block-end & inline-axis padding to match other browsers. r=emilio
You need to log in before you can comment on or make changes to this bug.