Bug 238356 – [css-cascade] "related-property" logic for (-webkit-)text-orientation is broken

• Component: CSS
• Status: RESOLVED FIXED

[Oriol Brufau]

Created attachment 455703 [details]
testcase

The text-orientation and -webkit-text-orientation properties are defined with the "related-property" flag.
They are two different longhands that share a computed value, so this flag tries to address the case where both properties are specified.
Typically, properties are applied in alphabetical order within their priority group, but "related-property" achieves that the last specified one wins:

  <div style="-webkit-text-orientation: mixed; text-orientation: upright">I'm upright</div>
  <div style="text-orientation: mixed; -webkit-text-orientation: upright">I'm upright</div>

However, it doesn't work here (see attached testcase):

  <style>
  div { -webkit-text-orientation: mixed }
  div { text-orientation: upright }
  </style>
  <div>I should be upright</div>

This case works well for (-webkit-)box-shadow, which instead of "related-property" is handled by shouldApplyPropertyInParseOrder().

I thought I could handle (-webkit-)text-orientation in the same way (bug 238350), but when shouldApplyPropertyInParseOrder() returns true, the property is deferred and applied after low-priority properties.
This conflicts with (-webkit-)text-orientation, which need to be high priority.

[Oriol Brufau]

For revert-layer this implies a problem like bug 238125 (but it would need to be addressed in a different way). This example is not upright:

  @layer { div { writing-mode: vertical-lr; text-orientation: upright; } }
  @layer { div { -webkit-text-orientation: revert-layer; } }

And CSSOM is also broken:

  CSS.supports("text-orientation", "sideways-right"); // false
  document.body.style.webkitTextOrientation = "sideways-right";
  document.body.style.textOrientation; // "sideways-right" !!! invalid value

Since the only difference between these properties is that -webkit-text-orientation accepts sideways-right, but this value always computes to sideways, I would:

 1. Make -webkit-text-orientation an shorthand of text-orientation
 2. Alias sideways-right into sideways at parse time

This is similar to the example in the spec https://drafts.csswg.org/css-cascade/#legacy-shorthand, which says that page-break-before:always expands to break-before:page at parse time.

[Oriol Brufau]

Created attachment 455718 [details]
Patch

[Oriol Brufau]

Created attachment 455750 [details]
Patch

[Oriol Brufau]

Created attachment 455751 [details]
Patch

[Oriol Brufau]

Created attachment 455754 [details]
Patch

[Oriol Brufau]

PTAL

[Radar WebKit Bug Importer]

<rdar://problem/91133741>

[Antti Koivisto]

Comment on attachment 455754 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455754&action=review

> Source/WebCore/css/parser/CSSPropertyParser.cpp:5659
> +    else
> +        return false;
> +    if (!keyword || !m_range.atEnd())
> +        return false;

could just remove the else brach, it will hit the !keyword case

[Oriol Brufau]

Created attachment 456807 [details]
Patch

[Oriol Brufau]

Comment on attachment 456807 [details]
Patch

(In reply to Antti Koivisto from comment #8)
> Comment on attachment 455754 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=455754&action=review
> 
> > Source/WebCore/css/parser/CSSPropertyParser.cpp:5659
> > +    else
> > +        return false;
> > +    if (!keyword || !m_range.atEnd())
> > +        return false;
> 
> could just remove the else brach, it will hit the !keyword case

Done.

[EWS]

Committed r292463 (249314@main): <https://commits.webkit.org/249314@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456807 [details].