writing mode needs to affect computed display

[bzbarsky]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1342739 and the following two testcases that stylo fails but gecko passes:

<div>
  <span style="writing-mode: vertical-rl">Text</span>
</div>
<script>
  document.write(getComputedStyle(document.querySelector("span")).display);
</script>

(should output "inline-block") and:

<div>
  <span style="writing-mode: vertical-rl; border: 20px solid green">Text</span>
</div>

(should not have the green border overlapping the text). That's because servo doesn't implement the provisions in https://drafts.csswg.org/css-writing-modes-3/#block-flow for boxes with specified display of "inline".

For stylo this should happen unconditionally. For servo, I guess this should happen only when "layout.writing-mode.enabled" is on? The rendering in that mode is way more broken than the stylo rendering anyway; there be problems there.

[bd339]

I would like to look into this, if I can get some help on how to get started 🙂

[emilio]

Hi @bd339, thanks for taking a look at this!

So, the specified display adjustment lives at components/style/properties/properties.mako.rs. There, we need to check if the specified display is inline and the parent containing block writing-mode is different from the current one, with something like:

if context.layout_parent_style.writing_mode != style.writing_mode && style.get_box().clone_display() == display::T::inline {
    style.mutate_box().set_display(display::T::inline_block);
}

Note that may be somewhat tricky because we need to skip text nodes from this, which is tricky because even though they inherit style from the parent, the writing mode should not change, but could with display: contents.

[bzbarsky]

I should note that there are some spec-side issues here too. I just sent https://lists.w3.org/Archives/Public/www-style/2017Mar/0045.html describing those. But it sounds like @emilio's suggestion matches Gecko which is about right for now.

[emilio]

Yeah, note that for this to match gecko, as noted in the email above, that piece of code should be under the existing display adjustments, and should call set_display (not set_adjusted_display), that is, also reset mOriginalDisplay. This bit is a bit fishy TBH :/

[bd339]

Thanks for pointing me in the right direction @bzbarsky and @emilio . I'm not sure I understand what you said about text nodes, so I'll show an example so you can correct me.

First off, if we assume display: contents is not a thing, skipping text nodes would happen automatically because they inherit writing-mode from their parent and wouldn't get through the if you posted? Is that what you said in the first part about text nodes?

Finally (because display: contents is a thing), would this be an example of what we need to watch out for?

<div>
  <span style="writing-mode: vertical-rl; display: contents">A text node</span>
</div>

As in: "A text node" inherits the vertical-rl writing-mode from the span but because of display: contents, it would be as if "A text node" is a direct child of the div, and its writing-mode would actually differ from its parent's?

[bzbarsky]

@bd339 That's correct.

For what it's worth, Gecko skips this writing mode fixup at this point in two situations: for text and for first-letter continuations. I think first-letter continuations can never have a writing-mode different from their "containing block", because they inherit either from the block directly or from the first-line, the "writing-mode" property does not apply to either first-letter or first-line, and things with "display:contents" don't generate first-letters. We should probably add asserts that we don't have a first-letter continuation ever hitting this case...