Bug 84543 – unicode-bidi:plaintext treats embedded element with unicode-bidi:plaintext or isolate incorrectly

• Component: CSS
• Status: NEW

[Aharon (Vladimir) Lanin]

The current definition of unicode-bidi:plaintext in CSS Writing Modes Level 3 states:

This value behaves as ‘isolate’ except that for the purposes of the Unicode bidirectional algorithm, the base directionality of each bidi paragraph immediately contained by the element is determined not by the element's computed ‘direction’ as usual, but by following the heuristic in rules P2 and P3 of the Unicode bidirectional algorithm. An element immediately contains a bidi paragraph if the element itself, but none of its descendants, both contains the entire bidi paragraph and is either a block container or a bidi-isolating inline.

Thus, if you have inline element A around inline element B, where A has unicode-bidi:plaintext and B has unicode-bidi:isolate, then:

1. The paragraphs in B must get their base direction from B's direction property as usual, not from their content as would be the case if B has unicode-bidi:plaintext - even though B is inside A and A does have unicode-bidi:plaintext. The "immediately contained" definition was recently added to clarify this very point.

2. B's content must not affect the direction of the paragraph in A inside which B occurs (because, according to the spec, "for the purpose of bidi resolution in its containing bidi paragraph (if any), [an element with unicode-bidi:isolate] is treated as if it were an Object Replacement Character (U+FFFC)"). The same is true if B also had unicode-bidi:plaintext (because unicode-bidi:plaintext "behaves as ‘isolate’").

Unfortunately, neither 1 nor 2 holds in WebKit. The first two attachments show this when A is a block element. (I used a <div> with unicode-bidi:-webkit-isolate style, but it could have been done with a <pre> out of the box.) The next two attachment show that things are even worse when A is an inline element: the content after B completely disappears. Very strange and worrisome.

[Aharon (Vladimir) Lanin]

Created attachment 138266 [details]
Test case 1: block plaintext around inline isolate.

[Aharon (Vladimir) Lanin]

Created attachment 138267 [details]
Test case 2: block plaintext around inline plaintext.

[Aharon (Vladimir) Lanin]

Created attachment 138268 [details]
Test case 3: inline plaintext around inline isolate.

[Aharon (Vladimir) Lanin]

Created attachment 138269 [details]
Test case 4: inline plaintext around inline plaintext.

[Levi Weintraub]

I hope to take a look at this next week.

[Aharon (Vladimir) Lanin]

FYI, this has been fixed in Blink. See https://code.google.com/p/chromium/issues/detail?id=276642, but watch out for https://code.google.com/p/chromium/issues/detail?id=299879 and https://code.google.com/p/chromium/issues/detail?id=301500.

[Igor Trindade Oliveira]

Created attachment 242055 [details]
Patch V1

[Igor Trindade Oliveira]

The patch is fixing just the test cases 1 and 2. The test cases 3 and 4 are a different bug.

[Igor Trindade Oliveira]

Created attachment 242060 [details]
Patch

[Igor Trindade Oliveira]

Created attachment 242067 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 242067 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:58
> +        if (isIsolated(current->style().unicodeBidi())

Maybe we want to stay inside root here?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:62
> +            else

I'm not sure this would ever occur. If I'm reading RenderBlockLineLayout correctly, every time we call this function, the root element's unicodeBidi() is Plaintext.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:78
> +            next = downcast<RenderElement>(*current).firstChild();

How do you know that this is a RenderElement?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:81
> +            while (current && current != root) {

Don't we have a better way of iterating through elements in order? Some sort of iterator class?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:101
> +    InlineIterator iter(rootElement, firstRenderObjectForDirectionalityDetermination(rootElement, iterator.renderer()), iterator.offset());

Is this really right? You are specifying the RenderObject input to InlineIterator, but keeping the original offset. What if the first RenderObject is shorter than the original iterator's offset?

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:104
> +    observer.setStatus(BidiStatus(rootElement->style().direction(), isOverride(rootElement->style().unicodeBidi())));

Using the root element here seems fishy to me. What if something between iter.renderer() and root has its direction specified?

[Myles C. Maxfield]

Comment on attachment 242067 [details]
Patch

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

>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:81
>> +            while (current && current != root) {
> 
> Don't we have a better way of iterating through elements in order? Some sort of iterator class?

RenderIterator

[Brent Fulgham]

Comment on attachment 242067 [details]
Patch

This patch can't be applied to current WebKit sources. Could you please address Myles' review comments, using RenderIterator, and upload a new patch?

[Ahmad Saleem]

I am able to reproduce this partially in Safari 15.5 on macOS 12.4.

Test Case 1 -> Reproducible:

Safari 15.5 - Does not match both boxes
Chrome Canary 104 - Matches both boxes
Firefox Nightly 103 - Matches both boxes

Test Case 1 -> Reproducible:

Safari 15.5 - Does not match both boxes
Chrome Canary 104 - Matches both boxes
Firefox Nightly 103 - Matches both boxes

Test Case 3 -> Matching Chrome but different from Firefox:

Safari 15.5 - Matches both boxes (Although in two lines)
Chrome Canary 104 - Matches both boxes (same as Safari)
Firefox Nightly 103 - Matches both boxes (In single line)

Test Case 4 -> Matching Chrome but different from Firefox:

Safari 15.5 - Matches both boxes (Although in two lines)
Chrome Canary 104 - Matches both boxes (same as Safari)
Firefox Nightly 103 - Matches both boxes (In single line)

If I am testing it incorrectly, please retest accordingly. Thanks!

[Myles C. Maxfield]

Thanks for retrying the reproduction cases!!! :D

[Radar WebKit Bug Importer]

<rdar://problem/98330916>

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #14)
> I am able to reproduce this partially in Safari 15.5 on macOS 12.4.
> 
> Test Case 1 -> Reproducible:
> 
> Safari 15.5 - Does not match both boxes
> Chrome Canary 104 - Matches both boxes
> Firefox Nightly 103 - Matches both boxes
> 
> Test Case 1 -> Reproducible:
> 
> Safari 15.5 - Does not match both boxes
> Chrome Canary 104 - Matches both boxes
> Firefox Nightly 103 - Matches both boxes
> 
> Test Case 3 -> Matching Chrome but different from Firefox:
> 
> Safari 15.5 - Matches both boxes (Although in two lines)
> Chrome Canary 104 - Matches both boxes (same as Safari)
> Firefox Nightly 103 - Matches both boxes (In single line)
> 
> Test Case 4 -> Matching Chrome but different from Firefox:
> 
> Safari 15.5 - Matches both boxes (Although in two lines)
> Chrome Canary 104 - Matches both boxes (same as Safari)
> Firefox Nightly 103 - Matches both boxes (In single line)
> 
> If I am testing it incorrectly, please retest accordingly. Thanks!

Update from Safari Technology Preview 153 (not checked compared to other browsers except where we were just matching one):

Test Case 1 -> No more reproducible.. :-)

STP 153 - Matches both boxes
Chrome Canary 104 - Matches both boxes
Firefox Nightly 103 - Matches both boxes

Test Case 1 -> No more reproducible.. :-)

STP 153 - Matches both boxes
Chrome Canary 104 - Matches both boxes
Firefox Nightly 103 - Matches both boxes

Test Case 3 -> Matching Chrome but different from Firefox: [No difference from last in STP 153]

Safari 15.5 - Matches both boxes (Although in two lines)
Chrome Canary 104 - Matches both boxes (same as Safari)
Firefox Nightly 103 - Matches both boxes (In single line)

Test Case 4 -> Matching Chrome but different from Firefox: [No difference from last in STP 153]

Safari 15.5 - Matches both boxes (Although in two lines)
Chrome Canary 104 - Matches both boxes (same as Safari)
Firefox Nightly 103 - Matches both boxes (In single line)