1483527 - Change fieldset's default padding to match other browsers

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Description

[Simon Pieters [:zcorpan]]

See
https://github.com/whatwg/html/pull/3922
https://github.com/web-platform-tests/wpt/pull/12501

Comment 1

[Daniel Holbert [:dholbert]]

This looks trivial - I can take it.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1483527: Change fieldset block-end & inline-axis padding to match other browsers.

Comment 3

[Daniel Holbert [:dholbert]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10edee9dabfb0eefe966257d022461d5bbc2f4fc

(I anticipate we'll need to update some tests' expectations)

Comment 4

[Daniel Holbert [:dholbert]]

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.

Comment 5

[Daniel Holbert [:dholbert]]

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 6

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d98e379dc38
Change fieldset block-end & inline-axis padding to match other browsers. r=emilio

Comment 8

[Andrei Ciure[:aciure]]

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

Comment 9

[Daniel Holbert [:dholbert]]

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.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b5fc64beafb
Change fieldset block-end & inline-axis padding to match other browsers. r=emilio

Comment 11

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/2b5fc64beafb