1134849 - frame construction for orthogonal inline elements

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Jonathan Kew [:jfkthame]]

We should check whether we're doing the right thing as regards frame construction when an inline element has an orthogonal writing mode; should it automagically be treated as inline-block in this case, or wrapped in an anonymous block, or what...?

Trivial example:

  data:text/html,<div>abc <b style="writing-mode:vertical-rl">hello world</b> xyz

Note that we currently differ from webkit/blink in how this is rendered.

This is likely relevant to implementing text-combine-horizontal (bug 1097499) properly, too.

Comment 1

[Jonathan Kew [:jfkthame]]

An additional example that currently shows incorrect behavior:

  data:text/html,<div style="writing-mode:vertical-rl">
    The <span style="writing-mode:initial">quick<b>brown</b>fox</span> jumps over the lazy dog.

Here, I believe the expected result would be for the phrase "quick<b>brown</b>fox" to appear on a single (horizontal) line within the vertical sentence. (Note that it doesn't even have internal spaces as potential line-break positions.) But we end up stacking the three horizontal textframes vertically.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: For display:inline elements whose writing mode is orthogonal to their parent's, the computed value should become inline-block

Seems like the main issue here is that orthogonal inline elements need to be treated as inline-block. Can we simply do this in nsStyleContext::ApplyStyleFixups, something like this? This makes computedStyle return inline-block for such elements, which also seems to be how Chrome handles it.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8576547 [details] [diff] [review]
For display:inline elements whose writing mode is orthogonal to their parent's, the computed value should become inline-block

This isn't quite right, because you should also be correcting disp->mOriginalDisplay when it is NS_STYLE_DISPLAY_INLINE and mDisplay is not, given that:
http://dev.w3.org/csswg/css2/visudet.html#abs-non-replaced-width says:

  The static-position containing block is the containing block of a hypothetical box that would have been the first box of the element if its specified 'position' value had been 'static' and its specified 'float' had been 'none'. (Note that due to the rules in section 9.7 this hypothetical calculation might require also assuming a different computed value for 'display'.) 

which is part of what mOriginalDisplay is for.

The other part of what mOriginalDisplay is for (and all of what mOriginalFloats is for) is doing correct computation when we start computation based on a cached struct in the rule tree (an aStartStruct parameter to nsRuleNode::Compute*Data); that's not relevant here since the GetUniqueStyleData means the data are cached on the style context and not the rule tree.  See bug 608756.

(I think you can assert that if mOriginalDisplay is not inline, then mDisplay is also not inline, so the initial test should be able to test only mOriginalDisplay.)


There should probably be a comment explaining this (like the comment above in the same function where we do the display fixups for the root).


It also seems like the root fixup, the flex/grid fixup, and the ruby fixup might be wrong in a similar way (although the latter two are not touching mOriginalDisplay at all, which I think they should be), although I didn't look that closely.



That said, I guess it's not  far enough from right that I'd object to landing it, so r=dbaron, although we should probably at least file a followup bug on getting mOriginalDisplay right for all of these display fixups.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Reftest for orthogonal inline elements

Here's a simple testcase to go with the patch here.

Comment 5

[Jonathan Kew [:jfkthame]]

Filed bug 1143302 about the mOriginalDisplay issues (comment 3 above).

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8577610 [details] [diff] [review]
Reftest for orthogonal inline elements

Seems like the <b>brown</b> instances in the reference should be display:inline rather than display:inline-block, although it probably doesn't actually change the rendering.

r=dbaron with that

Comment 7

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d525669326e
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ea22b9149a3

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1d525669326e
https://hg.mozilla.org/mozilla-central/rev/6ea22b9149a3