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

Source/core/rendering/RenderBlockLineLayout.cpp

 /*
  * Copyright (C) 2000 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2003, 2004, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All right reserved.
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 326 matching lines @@
                 extraWidth += borderPaddingMarginEnd(parentAsRenderInline);
             if (!start && !end)
                 return extraWidth;
         }
         child = parent;
         parent = child->parent();
     }
     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()) {
         if (iter.atParagraphSeparator())
             return;
         if (UChar current = iter.current()) {
             Direction charDirection = direction(current);
             if (charDirection == LeftToRight) {
                 dir = LTR;
                 return;
@@ skipping 912 matching lines @@
 
         // Only inlines make sense with unicode-bidi: isolate (blocks are already isolated).
         // FIXME: Because enterIsolate is not passed a RenderObject, we have to crawl up the
         // tree to see which parent inline is the isolate. We could change enterIsolate
         // to take a RenderObject and do this logic there, but that would be a layering
         // violation for BidiResolver (which knows nothing about RenderObject).
         RenderInline* isolatedInline = toRenderInline(containingIsolate(startObj, currentRoot));
 
         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();
         }
         isolatedResolver.setStatus(statusWithDirection(direction, isOverride(unicodeBidi)));
 
         setupResolverToResumeInIsolate(isolatedResolver, isolatedInline, startObj);
 
         // The starting position is the beginning of the first run within the isolate that was identified
         // during the earlier call to createBidiRunsForLine. This can be but is not necessarily the
@@ skipping 506 matching lines @@
 
         // This is a short-cut for empty lines.
         if (layoutState.lineInfo().isEmpty()) {
             if (lastRootBox())
                 lastRootBox()->setLineBreakInfo(end.m_obj, end.m_pos, resolver.status());
         } else {
             VisualDirectionOverride override = (styleToUse->rtlOrdering() == VisualOrder ? (styleToUse->direction() == LTR ? VisualLeftToRightOverride : VisualRightToLeftOverride) : NoVisualOverride);
 
             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.
             BidiRunList<BidiRun>& bidiRuns = resolver.runs();
             constructBidiRunsForLine(this, resolver, bidiRuns, end, override, layoutState.lineInfo().previousLineBrokeCleanly());
             ASSERT(resolver.position() == end);
 
             BidiRun* trailingSpaceRun = !layoutState.lineInfo().previousLineBrokeCleanly() ? handleTrailingSpaces(bidiRuns, resolver.context()) : 0;
 
             if (bidiRuns.runCount() && lineBreaker.lineWasHyphenated()) {
@@ skipping 466 matching lines @@
     layoutState.lineInfo().setPreviousLineBrokeCleanly(!last || last->endsWithBreak());
 
     if (last) {
         setLogicalHeight(last->lineBottomWithLeading());
         InlineIterator iter = InlineIterator(this, last->lineBreakObj(), last->lineBreakPos());
         resolver.setPosition(iter, numberOfIsolateAncestors(iter));
         resolver.setStatus(last->lineBreakBidiStatus());
     } 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));
     }
     return curr;
 }
 
 void RenderBlock::determineEndPosition(LineLayoutState& layoutState, RootInlineBox* startLine, InlineIterator& cleanLineStart, BidiStatus& cleanLineBidiStatus)
 {
     ASSERT(!layoutState.endLine());
@@ skipping 1175 matching lines @@
     lineGridBox->alignBoxesInBlockDirection(logicalHeight(), textBoxDataMap, verticalPositionCache);
 
     setLineGridBox(lineGridBox);
 
     // FIXME: If any of the characteristics of the box change compared to the old one, then we need to do a deep dirtying
     // (similar to what happens when the page height changes). Ideally, though, we only do this if someone is actually snapping
     // to this grid.
 }
 
 }