Title: [107000] trunk
Revision
107000
Author
le...@chromium.org
Date
2012-02-07 15:46:40 -0800 (Tue, 07 Feb 2012)

Log Message

unicode-bidi:plaintext is supposed to be effective on display:inline elements too
https://bugs.webkit.org/show_bug.cgi?id=73310

Reviewed by Eric Seidel.

Source/WebCore:

Adding support for unicode-bidi: plaintext as a property on inlines. These are treated
like unicode-bidi: isolate with the addition of their directionality being determined
by the UBA.

Tests: fast/text/international/inline-plaintext-is-isolated-expected.html
       fast/text/international/inline-plaintext-is-isolated.html
       fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html
       fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html
       fast/text/international/inline-plaintext-with-generated-content-expected.html
       fast/text/international/inline-plaintext-with-generated-content.html

* platform/text/UnicodeBidi.h:
(WebCore::isIsolated): Added this convenience function as Plaintext and Isolate Unicode-Bidi values
are both treated as isolated content.
* rendering/InlineIterator.h:
(WebCore::notifyObserverEnteredObject): Inline now supports Unicode-Bidi Plaintext.
(WebCore::notifyObserverWillExitObject): Ditto.
(WebCore::bidiFirstSkippingEmptyInlines): Changed to support being called without a resolver.
(WebCore::isIsolatedInline): Inline now supports Unicode-Bidi: Plaintext.
* rendering/RenderBlockLineLayout.cpp:
(WebCore::determineDirectionality): Generalized for inlines.
(WebCore::constructBidiRuns): Added support for Unicode-Bidi: Plaintext as an isolated inline.
(WebCore::RenderBlock::layoutRunsAndFloatsInRange): Fixed comment.
(WebCore::RenderBlock::determineStartPosition): Fixed comment and switched to updated
bidiFirstSkippingEmptyInlines.

LayoutTests:

Ref tests for unicode-bidi: plaintext on inlines.

* fast/text/international/inline-plaintext-is-isolated-expected.html: Added.
* fast/text/international/inline-plaintext-is-isolated.html: Added.
* fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html: Added.
* fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html: Added.
* fast/text/international/inline-plaintext-with-generated-content-expected.html: Added.
* fast/text/international/inline-plaintext-with-generated-content.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (106999 => 107000)


--- trunk/LayoutTests/ChangeLog	2012-02-07 23:43:28 UTC (rev 106999)
+++ trunk/LayoutTests/ChangeLog	2012-02-07 23:46:40 UTC (rev 107000)
@@ -1,3 +1,19 @@
+2012-02-07  Levi Weintraub  <le...@chromium.org>
+
+        unicode-bidi:plaintext is supposed to be effective on display:inline elements too
+        https://bugs.webkit.org/show_bug.cgi?id=73310
+
+        Reviewed by Eric Seidel.
+
+        Ref tests for unicode-bidi: plaintext on inlines.
+
+        * fast/text/international/inline-plaintext-is-isolated-expected.html: Added.
+        * fast/text/international/inline-plaintext-is-isolated.html: Added.
+        * fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html: Added.
+        * fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html: Added.
+        * fast/text/international/inline-plaintext-with-generated-content-expected.html: Added.
+        * fast/text/international/inline-plaintext-with-generated-content.html: Added.
+
 2012-02-07  Julien Chaffraix  <jchaffr...@webkit.org>
 
         Unreviewed gardening after r106982.

Added: trunk/LayoutTests/fast/text/international/inline-plaintext-is-isolated-expected.html (0 => 107000)


--- trunk/LayoutTests/fast/text/international/inline-plaintext-is-isolated-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/inline-plaintext-is-isolated-expected.html	2012-02-07 23:46:40 UTC (rev 107000)
@@ -0,0 +1,22 @@
+<!DOCTYPE HTML>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8">
+<style>
+  .border {
+    border: solid thin gray;
+  }
+</style>
+</head><body>
+  <table>
+    <tr>
+      <td class="border">1 <span dir="rtl">&#x05D0;!</span>&lrm; 2</td>
+      <td>should look the same as</td>
+      <td class="border">1 <span dir="rtl">&#x05D0;!</span>&lrm; 2</td>
+    </tr>
+    <tr>
+      <td class="border">1 a!<br><span dir="rtl">&#x05D0;!</span>&lrm; 2</td>
+      <td>should look the same as</td>
+      <td class="border">1 a!<br><span dir="rtl">&#x05D0;!</span>&lrm; 2</td>
+    </tr>
+  </table>
+</body></html>

Added: trunk/LayoutTests/fast/text/international/inline-plaintext-is-isolated.html (0 => 107000)


--- trunk/LayoutTests/fast/text/international/inline-plaintext-is-isolated.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/inline-plaintext-is-isolated.html	2012-02-07 23:46:40 UTC (rev 107000)
@@ -0,0 +1,29 @@
+<!DOCTYPE HTML>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8">
+<style>
+  .rtl {
+    direction: rtl;
+  }
+  .plaintext {
+    unicode-bidi: -webkit-plaintext;
+    unicode-bidi: plaintext;
+  }
+  .border {
+    border: solid thin gray;
+  }
+</style>
+</head><body>
+  <table>
+    <tr>
+      <td class="border">1 <span class="plaintext rtl">&#x05D0;!</span> 2</td>
+      <td>should look the same as</td>
+      <td class="border">1 <span dir="rtl">&#x05D0;!</span>&lrm; 2</td>
+    </tr>
+    <tr>
+      <td class="border">1 <span class="plaintext rtl">a!<br/>&#x05D0;!</span> 2</td>
+      <td>should look the same as</td>
+      <td class="border">1 a!<br><span dir="rtl">&#x05D0;!</span>&lrm; 2</td>
+    </tr>
+  </table>
+</body></html>

Added: trunk/LayoutTests/fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html (0 => 107000)


--- trunk/LayoutTests/fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html	2012-02-07 23:46:40 UTC (rev 107000)
@@ -0,0 +1,23 @@
+<!DOCTYPE HTML>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8">
+<style>
+  .border {
+    border: solid thin gray;
+    width: 20px;
+  }
+</style>
+</head><body>
+  <table>
+    <tr>
+      <td class="border"><span dir="rtl">1... .... ....<span>&#x05D0;</span></span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="rtl">1... .... ....<span>&#x05D0;</span></span></td>
+    </tr>
+    <tr>
+      <td class="border"><span dir="ltr">2... .... ....<span>a</span></span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="ltr">2... .... ....<span>a</span></span></td>
+    </tr>
+  </table>
+</body></html>

Added: trunk/LayoutTests/fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html (0 => 107000)


--- trunk/LayoutTests/fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html	2012-02-07 23:46:40 UTC (rev 107000)
@@ -0,0 +1,35 @@
+<!DOCTYPE HTML>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8">
+<style>
+  .plaintext {
+    unicode-bidi: -webkit-plaintext;
+    unicode-bidi: plaintext;
+  }
+  .border {
+    border: solid thin gray;
+    width: 20px;
+  }
+</style>
+<script>
+function runTest() {
+  var span1 = document.getElementById("replacement1");
+  span1.innerText = "א";
+  var span2 = document.getElementById("replacement2");
+  span2.innerText = "a";
+}
+</script>
+</head><body _onload_="runTest();">
+  <table>
+    <tr>
+      <td class="border"><span class="plaintext">1... .... ....<span id="replacement1">a</span></span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="rtl">1... .... ....<span>&#x05D0;</span></span></td>
+    </tr>
+    <tr>
+      <td class="border"><span class="plaintext">2... .... ....<span id="replacement2">&#x05D0;</span></span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="ltr">2... .... ....<span>a</span></span></td>
+    </tr>
+  </table>
+</body></html>

Added: trunk/LayoutTests/fast/text/international/inline-plaintext-with-generated-content-expected.html (0 => 107000)


--- trunk/LayoutTests/fast/text/international/inline-plaintext-with-generated-content-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/inline-plaintext-with-generated-content-expected.html	2012-02-07 23:46:40 UTC (rev 107000)
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8">
+<style>
+  td.border:first-letter {
+    color: red;
+  }
+  .border {
+    border: solid thin gray;
+    width: 20px;
+  }
+</style>
+</head><body>
+  <table>
+    <tr>
+      <td class="border"><span dir="ltr">a&#x05D0;!</span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="ltr">a&#x05D0;!</span></td>
+    </tr>
+    <tr>
+      <td class="border"><span dir="rtl">&#x05D0;a!</span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="rtl">&#x05D0;a!</span></td>
+    </tr>
+  </table>
+</body></html>

Added: trunk/LayoutTests/fast/text/international/inline-plaintext-with-generated-content.html (0 => 107000)


--- trunk/LayoutTests/fast/text/international/inline-plaintext-with-generated-content.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/inline-plaintext-with-generated-content.html	2012-02-07 23:46:40 UTC (rev 107000)
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html;charset=utf-8">
+<style>
+  .plaintext {
+    unicode-bidi: -webkit-plaintext;
+    unicode-bidi: plaintext;
+  }
+  td.border:first-letter {
+    color: red;
+  }
+  .border {
+    border: solid thin gray;
+    width: 20px;
+  }
+</style>
+</head><body>
+  <table>
+    <tr>
+      <td class="border"><span class="plaintext">a&#x05D0;!</span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="ltr">a&#x05D0;!</span></td>
+    </tr>
+    <tr>
+      <td class="border"><span class="plaintext">&#x05D0;a!</span></td>
+      <td>should look the same as</td>
+      <td class="border"><span dir="rtl">&#x05D0;a!</span></td>
+    </tr>
+  </table>
+</body></html>

Modified: trunk/Source/WebCore/ChangeLog (106999 => 107000)


--- trunk/Source/WebCore/ChangeLog	2012-02-07 23:43:28 UTC (rev 106999)
+++ trunk/Source/WebCore/ChangeLog	2012-02-07 23:46:40 UTC (rev 107000)
@@ -1,3 +1,36 @@
+2012-02-07  Levi Weintraub  <le...@chromium.org>
+
+        unicode-bidi:plaintext is supposed to be effective on display:inline elements too
+        https://bugs.webkit.org/show_bug.cgi?id=73310
+
+        Reviewed by Eric Seidel.
+
+        Adding support for unicode-bidi: plaintext as a property on inlines. These are treated
+        like unicode-bidi: isolate with the addition of their directionality being determined
+        by the UBA.
+
+        Tests: fast/text/international/inline-plaintext-is-isolated-expected.html
+               fast/text/international/inline-plaintext-is-isolated.html
+               fast/text/international/inline-plaintext-relayout-with-leading-neutrals-expected.html
+               fast/text/international/inline-plaintext-relayout-with-leading-neutrals.html
+               fast/text/international/inline-plaintext-with-generated-content-expected.html
+               fast/text/international/inline-plaintext-with-generated-content.html
+
+        * platform/text/UnicodeBidi.h:
+        (WebCore::isIsolated): Added this convenience function as Plaintext and Isolate Unicode-Bidi values
+        are both treated as isolated content.
+        * rendering/InlineIterator.h:
+        (WebCore::notifyObserverEnteredObject): Inline now supports Unicode-Bidi Plaintext.
+        (WebCore::notifyObserverWillExitObject): Ditto.
+        (WebCore::bidiFirstSkippingEmptyInlines): Changed to support being called without a resolver.
+        (WebCore::isIsolatedInline): Inline now supports Unicode-Bidi: Plaintext.
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::determineDirectionality): Generalized for inlines.
+        (WebCore::constructBidiRuns): Added support for Unicode-Bidi: Plaintext as an isolated inline.
+        (WebCore::RenderBlock::layoutRunsAndFloatsInRange): Fixed comment.
+        (WebCore::RenderBlock::determineStartPosition): Fixed comment and switched to updated
+        bidiFirstSkippingEmptyInlines.
+
 2012-02-07  Kentaro Hara  <hara...@chromium.org>
 
         [Refactoring] Use the [IsWorkerContext] IDL in CodeGeneratorV8.pm

Modified: trunk/Source/WebCore/platform/text/UnicodeBidi.h (106999 => 107000)


--- trunk/Source/WebCore/platform/text/UnicodeBidi.h	2012-02-07 23:43:28 UTC (rev 106999)
+++ trunk/Source/WebCore/platform/text/UnicodeBidi.h	2012-02-07 23:46:40 UTC (rev 107000)
@@ -34,8 +34,13 @@
     Override,
     Isolate,
     Plaintext
-};
+}; 
 
+inline bool isIsolated(const EUnicodeBidi& unicodeBidi)
+{
+    return unicodeBidi == Isolate || unicodeBidi == Plaintext;
 }
 
+}
+
 #endif

Modified: trunk/Source/WebCore/rendering/InlineIterator.h (106999 => 107000)


--- trunk/Source/WebCore/rendering/InlineIterator.h	2012-02-07 23:43:28 UTC (rev 106999)
+++ trunk/Source/WebCore/rendering/InlineIterator.h	2012-02-07 23:46:40 UTC (rev 107000)
@@ -131,14 +131,13 @@
         // Thus we ignore any possible dir= attribute on the span.
         return;
     }
-    if (unicodeBidi == Isolate) {
+    if (isIsolated(unicodeBidi)) {
         observer->enterIsolate();
         // Embedding/Override characters implied by dir= are handled when
         // we process the isolated span, not when laying out the "parent" run.
         return;
     }
 
-    // FIXME: Should unicode-bidi: plaintext really be embedding override/embed characters here?
     if (!observer->inIsolate())
         observer->embed(embedCharFromDirection(style->direction(), unicodeBidi), FromStyleOrDOM);
 }
@@ -152,7 +151,7 @@
     EUnicodeBidi unicodeBidi = object->style()->unicodeBidi();
     if (unicodeBidi == UBNormal)
         return; // Nothing to do for unicode-bidi: normal
-    if (unicodeBidi == Isolate) {
+    if (isIsolated(unicodeBidi)) {
         observer->exitIsolate();
         return;
     }
@@ -255,9 +254,8 @@
     return bidiNextShared(root, current, observer, IncludeEmptyInlines, endOfInlinePtr);
 }
 
-static inline RenderObject* bidiFirstSkippingEmptyInlines(RenderObject* root, InlineBidiResolver* resolver)
+static inline RenderObject* bidiFirstSkippingEmptyInlines(RenderObject* root, InlineBidiResolver* resolver = 0)
 {
-    ASSERT(resolver);
     RenderObject* o = root->firstChild();
     if (!o)
         return 0;
@@ -278,7 +276,8 @@
     if (o && !isIteratorTarget(o))
         o = bidiNextSkippingEmptyInlines(root, o, resolver);
 
-    resolver->commitExplicitEmbedding();
+    if (resolver)
+        resolver->commitExplicitEmbedding();
     return o;
 }
 
@@ -392,7 +391,7 @@
 static inline bool isIsolatedInline(RenderObject* object)
 {
     ASSERT(object);
-    return object->isRenderInline() && object->style()->unicodeBidi() == Isolate;
+    return object->isRenderInline() && isIsolated(object->style()->unicodeBidi());
 }
 
 static inline RenderObject* containingIsolate(RenderObject* object, RenderObject* root)

Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (106999 => 107000)


--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2012-02-07 23:43:28 UTC (rev 106999)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2012-02-07 23:46:40 UTC (rev 107000)
@@ -258,7 +258,7 @@
     return extraWidth;
 }
 
-static void determineParagraphDirection(TextDirection& dir, InlineIterator iter)
+static void determineDirectionality(TextDirection& dir, InlineIterator iter)
 {
     while (!iter.atEnd()) {
         if (iter.atParagraphSeparator())
@@ -970,18 +970,27 @@
         // 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* isolatedSpan = toRenderInline(containingIsolate(startObj, currentRoot));
+        RenderInline* isolatedInline = toRenderInline(containingIsolate(startObj, currentRoot));
         InlineBidiResolver isolatedResolver;
-        isolatedResolver.setStatus(statusWithDirection(isolatedSpan->style()->direction()));
+        EUnicodeBidi unicodeBidi = isolatedInline->style()->unicodeBidi();
+        TextDirection direction;
+        if (unicodeBidi == Plaintext)
+            determineDirectionality(direction, InlineIterator(isolatedInline, isolatedRun->object(), 0));
+        else {
+            ASSERT(unicodeBidi == Isolate);
+            direction = isolatedInline->style()->direction();
+        }
+        isolatedResolver.setStatus(statusWithDirection(direction));
 
         // FIXME: The fact that we have to construct an Iterator here
         // currently prevents this code from moving into BidiResolver.
-        if (!bidiFirstSkippingEmptyInlines(isolatedSpan, &isolatedResolver))
+        if (!bidiFirstSkippingEmptyInlines(isolatedInline, &isolatedResolver))
             continue;
+
         // 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
         // first run within the isolate.
-        InlineIterator iter = InlineIterator(isolatedSpan, startObj, isolatedRun->m_start);
+        InlineIterator iter = InlineIterator(isolatedInline, startObj, isolatedRun->m_start);
         isolatedResolver.setPositionIgnoringNestedIsolates(iter);
 
         // We stop at the next end of line; we may re-enter this isolate in the next call to constructBidiRuns().
@@ -1240,7 +1249,7 @@
         FloatingObject* lastFloatFromPreviousLine = (m_floatingObjects && !m_floatingObjects->set().isEmpty()) ? m_floatingObjects->set().last() : 0;
         end = lineBreaker.nextLineBreak(resolver, layoutState.lineInfo(), lineBreakIteratorInfo, lastFloatFromPreviousLine, consecutiveHyphenatedLines);
         if (resolver.position().atEnd()) {
-            // FIXME: We shouldn't be creating any runs in findNextLineBreak to begin with!
+            // FIXME: We shouldn't be creating any runs in nextLineBreak to begin with!
             // Once BidiRunList is separated from BidiResolver this will not be needed.
             resolver.runs().deleteRuns();
             resolver.markCurrentRunEmpty(); // FIXME: This can probably be replaced by an ASSERT (or just removed).
@@ -1259,7 +1268,7 @@
 
             if (isNewUBAParagraph && styleToUse->unicodeBidi() == Plaintext && !resolver.context()->parent()) {
                 TextDirection direction = styleToUse->direction();
-                determineParagraphDirection(direction, resolver.position());
+                determineDirectionality(direction, resolver.position());
                 resolver.setStatus(BidiStatus(direction, styleToUse->unicodeBidi() == Override));
             }
             // FIXME: This ownership is reversed. We should own the BidiRunList and pass it to createBidiRunsForLine.
@@ -1668,10 +1677,8 @@
         resolver.setStatus(last->lineBreakBidiStatus());
     } else {
         TextDirection direction = style()->direction();
-        if (style()->unicodeBidi() == Plaintext) {
-            // FIXME: Why does "unicode-bidi: plaintext" bidiFirstIncludingEmptyInlines when all other line layout code uses bidiFirstSkippingEmptyInlines?
-            determineParagraphDirection(direction, InlineIterator(this, bidiFirstIncludingEmptyInlines(this), 0));
-        }
+        if (style()->unicodeBidi() == Plaintext)
+            determineDirectionality(direction, InlineIterator(this, bidiFirstSkippingEmptyInlines(this), 0));
         resolver.setStatus(BidiStatus(direction, style()->unicodeBidi() == Override));
         InlineIterator iter = InlineIterator(this, bidiFirstSkippingEmptyInlines(this, &resolver), 0);
         resolver.setPosition(iter, numberOfIsolateAncestors(iter));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to