See the testcase for bug 1102175

To fix this, we need to make the CSS styling of <hr>, found in html.css[1], responsive to the writing mode. In a vertical document, adding hr { height: auto; width: 2px; margin: auto 0.5em auto 0.5em; } to override the values from html.css goes a long way towards "fixing" this, but it's not clear to me how best to make this happen automatically. Bug 1083134 would help here, by allowing us to specify the margin in logical terms. What about the height/width? We don't have (-moz-)block-size and (-moz-)inline-size properties... [1] http://hg.mozilla.org/mozilla-central/annotate/d1adecc7adad/layout/style/html.css#l602

(In reply to Jonathan Kew (:jfkthame) from comment #1) > We don't have (-moz-)block-size and > (-moz-)inline-size properties... In theory, another approach might be to have a :(-moz-)vertical, or :(-moz-)orientation (vertical/horizontal) CSS selector. I'm not sure how practical or useful that is, but I'm just throwing out the idea.

(In reply to Simon Montagu :smontagu from comment #2) > (In reply to Jonathan Kew (:jfkthame) from comment #1) > > We don't have (-moz-)block-size and > > (-moz-)inline-size properties... > > In theory, another approach might be to have a :(-moz-)vertical, or > :(-moz-)orientation (vertical/horizontal) CSS selector. I'm not sure how > practical or useful that is, but I'm just throwing out the idea. Yes, that could be useful in many situations, I suspect; but is a CSS selector allowed to depend on the value of a CSS property? That sounds hairy.... div:vertical { writing-mode: horizontal-tb; } div:horizontal { writing-mode: vertical-rl; } :)

As a first step here, simply removing the height property from <hr> in html.css will allow the rule to become a vertical line in vertical writing-mode contexts. This doesn't address the fact that the margins still need to be swapped, but at least the rule doesn't (virtually) vanish any more. Until we have the necessary logical-margin properties, I suppose vertical-page authors can explicitly change the margins on <hr> as needed, though that's not an adequate solution for content that needs to adapt to both vertical and horizontal contexts.

Note that without any explicit width or height, the <hr> element still ends up (by default) with a height of 2px in horizontal mode, and a width of 2px in vertical mode, because of its 1px-thick border. So the explicit height was redundant, AFAICT. Tryserver says this change doesn't break any existing tests: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=223d32ad4c62.

(In reply to Jonathan Kew (:jfkthame) from comment #1) > Bug 1083134 would help here, by allowing us to specify the margin in logical > terms. What about the height/width? We don't have (-moz-)block-size and > (-moz-)inline-size properties... And given that we don't actually need to specify height or width, it looks like bug 1083134 is the remaining blocker here: with logical margins that respect vertical writing mode, we'd be able to style <hr> with full writing-mode flexibility.

Comment on attachment 8530542 [details] [diff] [review] Don't apply a fixed height to <hr> elements Review of attachment 8530542 [details] [diff] [review]: ----------------------------------------------------------------- I'm not confident enough to r+ this, I think dbaron or bz should look at it.

Comment on attachment 8530542 [details] [diff] [review] Don't apply a fixed height to <hr> elements Given the box-sizing: border-box and the border, this doesn't change the default rendering. I consider the risk of the resulting getComputedStyle difference low.

FWIW, we differ from webkit on getComputedStyle here anyway, which suggests the web is unlikely to depend too heavily on it. https://hg.mozilla.org/integration/mozilla-inbound/rev/88befbeb7321

hey johnathan, this caused https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4317286&repo=mozilla-inbound and i had to back this out maybe we need to change the test there ?

I removed the asserts(2) annotation from the test that now passes without asserting; and also duplicated the test and added an explicit height to the <hr> element. This second version of the test still asserts, just as before, so we're still exercising the same codepath with the same unexpected condition. https://hg.mozilla.org/integration/mozilla-inbound/rev/7873b21367db