Closed Bug 1377188 Opened 5 years ago Closed 5 years ago

Change the UA 'box-sizing' style to 'content-box' for <hr>


(Core :: Layout, enhancement, P3)




Tracking Status
firefox57 --- fixed


(Reporter: jwatt, Assigned: jwatt)




(1 file)

While reviewing bug 1356114 I noticed that other browsers (at least Edge and Chrome) appear to style hr elements with 'box-sizing: content-box' whereas we use 'box-sizing: border-box'.
It seems a good-first-bug for layout ICs. WDYT?
Priority: -- → P3
Attached patch patchSplinter Review
Attachment #8890409 - Flags: review?(dholbert)
(In reply to Astley Chen [:astley] from comment #1)
> It seems a good-first-bug for layout ICs. WDYT?

Maybe, although they could practically just be pointed to the code to change, so I'm not sure there would be much learning value. At any rate, I'd like to fix this before we land anything for bug 1356114, so I'm just going to fix this.
Comment on attachment 8890409 [details] [diff] [review]

Review of attachment 8890409 [details] [diff] [review]:

Could you include a reftest? e.g. I think the following testcases should render the same:
 data:text/html,<hr style="height: 4px; background: yellow; border: 1px solid black">
 data:text/html,<div style="height: 4px; background: yellow; border: 1px solid black">

(the only difference being "hr" vs. "div")

In Chrome, those two render the same right now, and I presume they will in Firefox after your patch, too. (they don't match in Firefox right now, due to the box-sizing difference)

r=me assuming a reasonable testcase
Attachment #8890409 - Flags: review?(dholbert) → review+
I did a due-dilligence bit of archeology to check when this "border-box" styling was added to <hr>, too. It was back in 2003, as part of bug 38370.  There was no discussion of "box-sizing" on the bug, so I don't know precisely why -- but there is a mention of "inset" borders, so I'm guessing this "box-sizing:border-box" was part of making those inset-looking borders also "inset" from a layout calculations perspective, too.)

Here is the commit in the gecko-dev repo (imported from CVS):

Anyway -- as long as this is improving webcompat (and it seems like it is), this seems like an obvious win.
Pushed by
part 1 - Change the UA 'box-sizing' style to 'content-box' for <hr>. r=dholbert
part 2 - Add a reftest to check the default value of 'box-sizing' on <hr>. r=dholbert
Thanks for that digging. I followed the history back far enough to know that it predated the move from CVS to hg, but the extra background is good to have.
Pushed by
part 3 - Mark WPT's the-hr-element-0/width.html test as passing on Mac. r=orange CLOSEDTREE
Duplicate of this bug: 1163906
You need to log in before you can comment on or make changes to this bug.