unicode-bidi: plaintext is treating embedded unicode-bidi: isolate incorrectly.

Source/core/rendering/RenderBlockLineLayout.cpp

Index: Source/core/rendering/RenderBlockLineLayout.cpp
diff --git a/Source/core/rendering/RenderBlockLineLayout.cpp b/Source/core/rendering/RenderBlockLineLayout.cpp
index b7484d68e91b45e2ddf2eb0484dcab30d05289b9..0e1a0aa1b69368d12b8d65c51597f78dab21c83d 100644
--- a/Source/core/rendering/RenderBlockLineLayout.cpp
+++ b/Source/core/rendering/RenderBlockLineLayout.cpp
@@ -344,6 +344,50 @@ static LayoutUnit inlineLogicalWidth(RenderObject* child, bool start = true, boo
     return extraWidth;
 }
 
+static inline RenderObject* bidiFirstSkippingIsolated(RenderObject* root, RenderObject* current = 0)

[eseidel, 2013/08/20 23:01:37]

Bleh.  All these bidiFirst* functions are ugly.  Is there no nice way to share
code here?  I know that I split these because bidiFirst used to have a bunch of
differnet modes which didn't make sense to share code together.  But adding
another copy of this function smells bad.

[leviw_travelin_and_unemployed, 2013/08/21 02:04:37]

I just talked to Eric about this, and this seems like a different problem being
solved than bidiFirst.

It seems to me that determineDirectionality should be smart enough (and perhaps
renamed to indicate this intelligence) to do this work. Unless you feel there's
a case where it shouldn't behave this way, in which case I'm happy to listen to
that argument.

+{
+    RenderObject* next = current;
+
+    while (current) {
+        if (isIsolated(current->style()->unicodeBidi())
+            && (current->isRenderInline() || current->isRenderBlock())) {
+            if (current != root)
+                current = 0;
+            else
+                current = next;
+            break;
+        }
+        current = current->parent();
+    }
+
+    if (!current)
+        current = root->firstChild();
+
+    while (current) {
+        next = 0;
+        if (isIteratorTarget(current) && !(current->isText() && toRenderText(current)->isAllCollapsibleWhitespace()))
+            break;
+
+        if (!isIteratorTarget(current) && !isIsolated(current->style()->unicodeBidi()))
+            next = current->firstChild();
+
+        if (!next) {
+            while (current && current != root) {
+                next = current->nextSibling();
+                if (next)
+                    break;
+                current = current->parent();
+            }
+        }
+
+        if (!next)
+            break;
+
+        current = next;
+    }
+    return current;
+}
+
 static void determineDirectionality(TextDirection& dir, InlineIterator iter)
 {
     while (!iter.atEnd()) {
@@ -1276,9 +1320,9 @@ static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver,
 
         InlineBidiResolver isolatedResolver;
         EUnicodeBidi unicodeBidi = isolatedInline->style()->unicodeBidi();
-        TextDirection direction;
+        TextDirection direction = isolatedInline->style()->direction();
         if (unicodeBidi == Plaintext)
-            determineDirectionality(direction, InlineIterator(isolatedInline, isolatedRun->object(), 0));
+            determineDirectionality(direction, InlineIterator(isolatedInline, bidiFirstSkippingIsolated(isolatedInline, startObj), 0));
         else {
             ASSERT(unicodeBidi == Isolate || unicodeBidi == IsolateOverride);
             direction = isolatedInline->style()->direction();
@@ -1805,7 +1849,7 @@ void RenderBlock::layoutRunsAndFloatsInRange(LineLayoutState& layoutState, Inlin
 
             if (isNewUBAParagraph && styleToUse->unicodeBidi() == Plaintext && !resolver.context()->parent()) {
                 TextDirection direction = styleToUse->direction();
-                determineDirectionality(direction, resolver.position());
+                determineDirectionality(direction, InlineIterator(resolver.position().root(), bidiFirstSkippingIsolated(resolver.position().root(), resolver.position().object()), resolver.position().offset()));
                 resolver.setStatus(BidiStatus(direction, isOverride(styleToUse->unicodeBidi())));
             }
             // FIXME: This ownership is reversed. We should own the BidiRunList and pass it to createBidiRunsForLine.
@@ -2292,7 +2336,7 @@ RootInlineBox* RenderBlock::determineStartPosition(LineLayoutState& layoutState,
     } else {
         TextDirection direction = style()->direction();
         if (style()->unicodeBidi() == Plaintext)
-            determineDirectionality(direction, InlineIterator(this, bidiFirstSkippingEmptyInlines(this), 0));
+            determineDirectionality(direction, InlineIterator(this, bidiFirstSkippingIsolated(this), 0));
         resolver.setStatus(BidiStatus(direction, isOverride(style()->unicodeBidi())));
         InlineIterator iter = InlineIterator(this, bidiFirstSkippingEmptyInlines(this, &resolver), 0);
         resolver.setPosition(iter, numberOfIsolateAncestors(iter));