Closed Bug 1377188 Opened 5 years ago Closed 5 years ago
Change the UA 'box-sizing' style to 'content-box' for <hr>
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?
Status: NEW → ASSIGNED
Priority: -- → P3
(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] 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
Attachment #8890409 - Flags: review?(dholbert) → review+
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.
Pushed by email@example.com: 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
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 firstname.lastname@example.org: 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
You need to log in before you can comment on or make changes to this bug.