Closed Bug 1483527 Opened 2 years ago Closed 2 years ago

Change fieldset's default padding to match other browsers

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: zcorpan, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This looks trivial - I can take it.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Priority: -- → P3
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10edee9dabfb0eefe966257d022461d5bbc2f4fc

(I anticipate we'll need to update some tests' expectations)
Flags: needinfo?(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.
Depends on: 234740
Flags: needinfo?(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 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+
Blocks: fieldset
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3d98e379dc38
Change fieldset block-end & inline-axis padding to match other browsers. r=emilio
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 dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2b5fc64beafb
Change fieldset block-end & inline-axis padding to match other browsers. r=emilio
Flags: needinfo?(dholbert)
https://hg.mozilla.org/mozilla-central/rev/2b5fc64beafb
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.