Closed Bug 1483527 Opened 2 years ago Closed 2 years ago

Change fieldset's default padding to match other browsers


(Core :: Layout, enhancement, P3)




Tracking Status
firefox63 --- fixed


(Reporter: zcorpan, Assigned: dholbert)


(Blocks 1 open bug)



(1 file)

This looks trivial - I can take it.
Assignee: nobody → dholbert
Priority: -- → P3
Try run:

(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 where we made a tweak to match IE on WinXP.

Here's that patch: attachment 143216 [details] [diff] [review] /

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).

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
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
Change fieldset block-end & inline-axis padding to match other browsers. r=emilio
Flags: needinfo?(dholbert)
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.