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

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Description

[Jonathan Watt [:jwatt]]

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

Comment 1

[Astley Chen (inactive)]

It seems a good-first-bug for layout ICs. WDYT?

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 3

[Jonathan Watt [:jwatt]]

(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 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8890409 [details] [diff] [review]
patch

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

Comment 5

[Daniel Holbert [:dholbert]]

I did a due-dilligence bit of searchfox.org 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):
  https://github.com/mozilla/gecko-dev/commit/a9e2dece768571828947e56bdbd23abd3d0b24f9

Anyway -- as long as this is improving webcompat (and it seems like it is), this seems like an obvious win.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20fe3c8c0540
part 1 - Change the UA 'box-sizing' style to 'content-box' for <hr>. r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/606245cf6527
part 2 - Add a reftest to check the default value of 'box-sizing' on <hr>. r=dholbert

Comment 7

[Jonathan Watt [:jwatt]]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b49898ac9063
part 3 - Mark WPT's the-hr-element-0/width.html test as passing on Mac. r=orange CLOSEDTREE

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/20fe3c8c0540
https://hg.mozilla.org/mozilla-central/rev/606245cf6527
https://hg.mozilla.org/mozilla-central/rev/b49898ac9063