Changeset 109806 in webkit

Timestamp:

Mar 5, 2012 3:09:22 PM (12 years ago)

Author:

rniwa@webkit.org

Message:

unicode-bidi should support isolate override and override isolate
​https://bugs.webkit.org/show_bug.cgi?id=73164

Reviewed by Eric Seidel.

Source/WebCore:

Updated CSS parser and CSS style selector to support the union of bidi-override and isolate in
unicode-bidi property. Added OverrideIsolate to EUnicodeBidi instead of turning Override and Isolate
into bit flags to avoid increasing the number of bits required to store the unicodeBidi flag.

Also fixed a bug in RenderBlock::constructTextRun to actually check whether an isolated run's direction
is overridden or not when constructing one.

Tests: fast/css/unicode-bidi-computed-value.html

fast/text/bidi-override-isolate.html

• css/CSSComputedStyleDeclaration.cpp:

(WebCore::renderUnicodeBidiFlagsToCSSValue): Added; Create a CSSValueList when unicode-bidi has both
isolate and bidi-override specified.
(WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue): Calls renderUnicodeBidiFlagsToCSSValue.

• css/CSSParser.cpp:

(WebCore::CSSParser::parseValue): Support parsing combinations of -webkit-isolate and bidi-override;
Create a CSSValueList in such cases.

• css/CSSPrimitiveValueMappings.h:

(WebCore):

• css/CSSStyleApplyProperty.cpp:

(ApplyPropertyUnicodeBidi):
(WebCore::ApplyPropertyUnicodeBidi::applyValue): Support combinations of -webkit-isolate and
bidi-override. Set the unicodeBidi flag to OverrideIsolate in such cases.
(WebCore::ApplyPropertyUnicodeBidi::createHandler):
(WebCore):
(WebCore::CSSStyleApplyProperty::CSSStyleApplyProperty):

• html/canvas/CanvasRenderingContext2D.cpp:

(WebCore::CanvasRenderingContext2D::drawTextInternal):

• platform/text/UnicodeBidi.h: Added OverrideIsolate. We don't use bit flags to avoid increasing the

number of bits required to store flags especially because isolate and bidi-override are only values
that can be combined.
(WebCore::isIsolated):
(WebCore):
(WebCore::isOverride):

• rendering/RenderBlock.cpp:

(WebCore::RenderBlock::constructTextRun):

• rendering/RenderBlockLineLayout.cpp:

(WebCore::statusWithDirection): Takes isOverride; we used to assume it's always false.
(WebCore::constructBidiRuns): Instantiate isolatedResolver with a proper value of isOverride.
(WebCore::RenderBlock::layoutRunsAndFloatsInRange):
(WebCore::RenderBlock::determineStartPosition):

• rendering/RenderListBox.cpp:

(WebCore::RenderListBox::paintItemForeground):

• rendering/RenderMenuList.cpp:

(WebCore::RenderMenuList::itemStyle):
(WebCore::RenderMenuList::menuStyle):

• rendering/RenderTextControlSingleLine.cpp:

(WebCore::RenderTextControlSingleLine::menuStyle):

• rendering/svg/SVGTextMetrics.cpp:

(WebCore::SVGTextMetrics::constructTextRun):

LayoutTests:

Added a test for retrieving computed value of unicode-bidi property
and also added a test to render "unicode-bidi: -webkit-isolate bidi-override".

• fast/css/unicode-bidi-computed-value-expected.txt: Added.
• fast/css/unicode-bidi-computed-value.html: Added.
• fast/text/bidi-override-isolate-expected.html: Added.
• fast/text/bidi-override-isolate.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/unicode-bidi-computed-value-expected.txt (added)
• LayoutTests/fast/css/unicode-bidi-computed-value.html (added)
• LayoutTests/fast/text/bidi-override-isolate-expected.html (added)
• LayoutTests/fast/text/bidi-override-isolate.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSComputedStyleDeclaration.cpp (modified) (2 diffs)
• Source/WebCore/css/CSSParser.cpp (modified) (1 diff)
• Source/WebCore/css/CSSPrimitiveValueMappings.h (modified) (1 diff)
• Source/WebCore/css/CSSStyleApplyProperty.cpp (modified) (2 diffs)
• Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp (modified) (1 diff)
• Source/WebCore/platform/text/UnicodeBidi.h (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockLineLayout.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderListBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderMenuList.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderTextControlSingleLine.cpp (modified) (1 diff)
• Source/WebCore/rendering/svg/SVGTextMetrics.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-03-02  Ryosuke Niwa  <rniwa@webkit.org>
+
+        unicode-bidi should support isolate override and override isolate
+        https://bugs.webkit.org/show_bug.cgi?id=73164
+
+        Reviewed by Eric Seidel.
+
+        Added a test for retrieving computed value of unicode-bidi property
+        and also added a test to render "unicode-bidi: -webkit-isolate bidi-override".
+
+        * fast/css/unicode-bidi-computed-value-expected.txt: Added.
+        * fast/css/unicode-bidi-computed-value.html: Added.
+        * fast/text/bidi-override-isolate-expected.html: Added.
+        * fast/text/bidi-override-isolate.html: Added.
+
 2012-03-05  Tony Chang  <tony@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        unicode-bidi should support isolate override and override isolate
+        https://bugs.webkit.org/show_bug.cgi?id=73164
+
+        Reviewed by Eric Seidel.
+
+        Updated CSS parser and CSS style selector to support the union of bidi-override and isolate in
+        unicode-bidi property. Added OverrideIsolate to EUnicodeBidi instead of turning Override and Isolate
+        into bit flags to avoid increasing the number of bits required to store the unicodeBidi flag.
+
+        Also fixed a bug in RenderBlock::constructTextRun to actually check whether an isolated run's direction
+        is overridden or not when constructing one.
+
+        Tests: fast/css/unicode-bidi-computed-value.html
+               fast/text/bidi-override-isolate.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::renderUnicodeBidiFlagsToCSSValue): Added; Create a CSSValueList when unicode-bidi has both
+        isolate and bidi-override specified.
+        (WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue):  Calls renderUnicodeBidiFlagsToCSSValue.
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseValue): Support parsing combinations of -webkit-isolate and bidi-override;
+        Create a CSSValueList in such cases.
+        * css/CSSPrimitiveValueMappings.h:
+        (WebCore):
+        * css/CSSStyleApplyProperty.cpp:
+        (ApplyPropertyUnicodeBidi):
+        (WebCore::ApplyPropertyUnicodeBidi::applyValue): Support combinations of -webkit-isolate and
+        bidi-override. Set the unicodeBidi flag to OverrideIsolate in such cases.
+        (WebCore::ApplyPropertyUnicodeBidi::createHandler):
+        (WebCore):
+        (WebCore::CSSStyleApplyProperty::CSSStyleApplyProperty):
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::drawTextInternal):
+        * platform/text/UnicodeBidi.h: Added OverrideIsolate. We don't use bit flags to avoid increasing the
+        number of bits required to store flags especially because isolate and bidi-override are only values
+        that can be combined.
+        (WebCore::isIsolated):
+        (WebCore):
+        (WebCore::isOverride):
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::constructTextRun):
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::statusWithDirection): Takes isOverride; we used to assume it's always false.
+        (WebCore::constructBidiRuns): Instantiate isolatedResolver with a proper value of isOverride.
+        (WebCore::RenderBlock::layoutRunsAndFloatsInRange):
+        (WebCore::RenderBlock::determineStartPosition):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::paintItemForeground):
+        * rendering/RenderMenuList.cpp:
+        (WebCore::RenderMenuList::itemStyle):
+        (WebCore::RenderMenuList::menuStyle):
+        * rendering/RenderTextControlSingleLine.cpp:
+        (WebCore::RenderTextControlSingleLine::menuStyle):
+        * rendering/svg/SVGTextMetrics.cpp:
+        (WebCore::SVGTextMetrics::constructTextRun):
+
 2012-03-05  Emil A Eklund  <eae@chromium.org>
 

trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

@@ -1129 +1129 @@
 }
 
+static PassRefPtr<CSSValue> renderUnicodeBidiFlagsToCSSValue(EUnicodeBidi unicodeBidi, CSSValuePool* cssValuePool)
+{
+    switch (unicodeBidi) {
+    case UBNormal:
+        return cssValuePool->createIdentifierValue(CSSValueNormal);
+    case Embed:
+        return cssValuePool->createIdentifierValue(CSSValueEmbed);
+    case Plaintext:
+        return cssValuePool->createIdentifierValue(CSSValueWebkitPlaintext);
+    case Override:
+        return cssValuePool->createIdentifierValue(CSSValueBidiOverride);
+    case Isolate:
+        return cssValuePool->createIdentifierValue(CSSValueWebkitIsolate);
+    case OverrideIsolate:
+    {
+        RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
+        list->append(cssValuePool->createIdentifierValue(CSSValueBidiOverride));
+        list->append(cssValuePool->createIdentifierValue(CSSValueWebkitIsolate));
+        return list;
+    }
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
 static PassRefPtr<CSSValue> renderTextDecorationFlagsToCSSValue(int textDecoration, CSSValuePool* cssValuePool)
 {
@@ -1898 +1923 @@
             return getPositionOffsetValue(style.get(), CSSPropertyTop, cssValuePool);
         case CSSPropertyUnicodeBidi:
-            return cssValuePool->createValue(style->unicodeBidi());
+            return renderUnicodeBidiFlagsToCSSValue(style->unicodeBidi(), cssValuePool);
         case CSSPropertyVerticalAlign:
             switch (style->verticalAlign()) {

trunk/Source/WebCore/css/CSSParser.cpp

@@ -969 +969 @@
             return parseQuotes(propId, important);
         break;
-    case CSSPropertyUnicodeBidi: // normal | embed | bidi-override | isolate | plaintext | inherit
+    case CSSPropertyUnicodeBidi: // normal | embed | (bidi-override || isolate) | plaintext | inherit
         if (id == CSSValueNormal
             || id == CSSValueEmbed
-            || id == CSSValueBidiOverride
-            || id == CSSValueWebkitIsolate
             || id == CSSValueWebkitPlaintext)
             validPrimitive = true;
+        else {
+            RefPtr<CSSValueList> list = CSSValueList::createSpaceSeparated();
+            bool isValid = true;
+            while (isValid && value) {
+                switch (value->id) {
+                case CSSValueBidiOverride:
+                case CSSValueWebkitIsolate:
+                    list->append(cssValuePool()->createIdentifierValue(value->id));
+                    break;
+                default:
+                    isValid = false;
+                }
+                value = m_valueList->next();
+            }
+            if (list->length() && isValid) {
+                parsedValue = list.release();
+                m_valueList->next();
+            }
+        }
         break;
 

trunk/Source/WebCore/css/CSSPrimitiveValueMappings.h

@@ -2207 +2207 @@
 }
 
-template<> inline CSSPrimitiveValue::CSSPrimitiveValue(EUnicodeBidi e)
-    : CSSValue(PrimitiveClass)
-{
-    m_primitiveUnitType = CSS_IDENT;
-    switch (e) {
-    case UBNormal:
-        m_value.ident = CSSValueNormal;
-        break;
-    case Embed:
-        m_value.ident = CSSValueEmbed;
-        break;
-    case Override:
-        m_value.ident = CSSValueBidiOverride;
-        break;
-    case Isolate:
-        m_value.ident = CSSValueWebkitIsolate;
-        break;
-    case Plaintext:
-        m_value.ident = CSSValueWebkitPlaintext;
-        break;
-    }
-}
-
 template<> inline CSSPrimitiveValue::operator EUnicodeBidi() const
 {

trunk/Source/WebCore/css/CSSStyleApplyProperty.cpp

@@ -1141 +1141 @@
     {
         PropertyHandler handler = ApplyPropertyDefaultBase<ETextDecoration, &RenderStyle::textDecoration, ETextDecoration, &RenderStyle::setTextDecoration, ETextDecoration, &RenderStyle::initialTextDecoration>::createHandler();
+        return PropertyHandler(handler.inheritFunction(), handler.initialFunction(), &applyValue);
+    }
+};
+
+class ApplyPropertyUnicodeBidi {
+public:
+    static void applyValue(CSSStyleSelector* selector, CSSValue* value)
+    {
+        if (value->isValueList()) {
+            EUnicodeBidi rendererUnicodeBidi = RenderStyle::initialUnicodeBidi();
+            for (CSSValueListIterator i = value; i.hasMore(); i.advance()) {
+                CSSValue* item = i.value();
+                ASSERT(item->isPrimitiveValue());
+                EUnicodeBidi currentValue = *static_cast<CSSPrimitiveValue*>(item);
+                ASSERT(currentValue == Override || currentValue == Isolate);
+                if (currentValue != rendererUnicodeBidi && rendererUnicodeBidi != RenderStyle::initialUnicodeBidi())
+                    rendererUnicodeBidi = OverrideIsolate;
+                else
+                    rendererUnicodeBidi = currentValue;
+            }
+            selector->style()->setUnicodeBidi(rendererUnicodeBidi);
+        }
+        if (!value->isPrimitiveValue())
+            return;
+        CSSPrimitiveValue* primitiveValue = static_cast<CSSPrimitiveValue*>(value);
+        selector->style()->setUnicodeBidi(*primitiveValue);
+    }
+    static PropertyHandler createHandler()
+    {
+        PropertyHandler handler = ApplyPropertyDefaultBase<EUnicodeBidi, &RenderStyle::unicodeBidi, EUnicodeBidi, &RenderStyle::setUnicodeBidi, EUnicodeBidi, &RenderStyle::initialUnicodeBidi>::createHandler();
         return PropertyHandler(handler.inheritFunction(), handler.initialFunction(), &applyValue);
     }
@@ -1795 +1825 @@
     setPropertyHandler(CSSPropertyTextTransform, ApplyPropertyDefault<ETextTransform, &RenderStyle::textTransform, ETextTransform, &RenderStyle::setTextTransform, ETextTransform, &RenderStyle::initialTextTransform>::createHandler());
     setPropertyHandler(CSSPropertyTop, ApplyPropertyLength<&RenderStyle::top, &RenderStyle::setTop, &RenderStyle::initialOffset, AutoEnabled>::createHandler());
-    setPropertyHandler(CSSPropertyUnicodeBidi, ApplyPropertyDefault<EUnicodeBidi, &RenderStyle::unicodeBidi, EUnicodeBidi, &RenderStyle::setUnicodeBidi, EUnicodeBidi, &RenderStyle::initialUnicodeBidi>::createHandler());
+    setPropertyHandler(CSSPropertyUnicodeBidi, ApplyPropertyUnicodeBidi::createHandler());
     setPropertyHandler(CSSPropertyVerticalAlign, ApplyPropertyVerticalAlign::createHandler());
     setPropertyHandler(CSSPropertyVisibility, ApplyPropertyDefault<EVisibility, &RenderStyle::visibility, EVisibility, &RenderStyle::setVisibility, EVisibility, &RenderStyle::initialVisibility>::createHandler());

trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp

@@ -2029 +2029 @@
     TextDirection direction = computedStyle ? computedStyle->direction() : LTR;
     bool isRTL = direction == RTL;
-    bool override = computedStyle ? computedStyle->unicodeBidi() == Override : false;
+    bool override = computedStyle ? isOverride(computedStyle->unicodeBidi()) : false;
 
     unsigned length = text.length();

trunk/Source/WebCore/platform/text/UnicodeBidi.h

@@ -34 +34 @@
     Override,
     Isolate,
-    Plaintext
+    Plaintext,
+    OverrideIsolate,
 }; 
 
 inline bool isIsolated(const EUnicodeBidi& unicodeBidi)
 {
-    return unicodeBidi == Isolate || unicodeBidi == Plaintext;
+    return unicodeBidi == Isolate || unicodeBidi == OverrideIsolate || unicodeBidi == Plaintext;
+}
+
+inline bool isOverride(EUnicodeBidi unicodeBidi)
+{
+    return unicodeBidi == Override || unicodeBidi == OverrideIsolate;
 }
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -7345 +7345 @@
             textDirection = style->direction();
         if (flags & RespectDirectionOverride)
-            directionalOverride |= style->unicodeBidi() == Override;
+            directionalOverride |= isOverride(style->unicodeBidi());
     }
 

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

@@ -944 +944 @@
 
 // FIXME: This should be a BidiStatus constructor or create method.
-static inline BidiStatus statusWithDirection(TextDirection textDirection)
+static inline BidiStatus statusWithDirection(TextDirection textDirection, bool isOverride)
 {
     WTF::Unicode::Direction direction = textDirection == LTR ? LeftToRight : RightToLeft;
-    RefPtr<BidiContext> context = BidiContext::create(textDirection == LTR ? 0 : 1, direction, false, FromStyleOrDOM);
+    RefPtr<BidiContext> context = BidiContext::create(textDirection == LTR ? 0 : 1, direction, isOverride, FromStyleOrDOM);
 
     // This copies BidiStatus and may churn the ref on BidiContext. I doubt it matters.
@@ -981 +981 @@
             determineDirectionality(direction, InlineIterator(isolatedInline, isolatedRun->object(), 0));
         else {
-            ASSERT(unicodeBidi == Isolate);
+            ASSERT(unicodeBidi == Isolate || unicodeBidi == OverrideIsolate);
             direction = isolatedInline->style()->direction();
         }
-        isolatedResolver.setStatus(statusWithDirection(direction));
+        isolatedResolver.setStatus(statusWithDirection(direction, isOverride(unicodeBidi)));
 
         // FIXME: The fact that we have to construct an Iterator here
@@ -1273 +1273 @@
                 TextDirection direction = styleToUse->direction();
                 determineDirectionality(direction, resolver.position());
-                resolver.setStatus(BidiStatus(direction, styleToUse->unicodeBidi() == Override));
+                resolver.setStatus(BidiStatus(direction, isOverride(styleToUse->unicodeBidi())));
             }
             // FIXME: This ownership is reversed. We should own the BidiRunList and pass it to createBidiRunsForLine.
@@ -1683 +1683 @@
         if (style()->unicodeBidi() == Plaintext)
             determineDirectionality(direction, InlineIterator(this, bidiFirstSkippingEmptyInlines(this), 0));
-        resolver.setStatus(BidiStatus(direction, style()->unicodeBidi() == Override));
+        resolver.setStatus(BidiStatus(direction, isOverride(style()->unicodeBidi())));
         InlineIterator iter = InlineIterator(this, bidiFirstSkippingEmptyInlines(this, &resolver), 0);
         resolver.setPosition(iter, numberOfIsolateAncestors(iter));

trunk/Source/WebCore/rendering/RenderListBox.cpp

@@ -410 +410 @@
     unsigned length = itemText.length();
     const UChar* string = itemText.characters();
-    TextRun textRun(string, length, false, 0, 0, TextRun::AllowTrailingExpansion, itemStyle->direction(), itemStyle->unicodeBidi() == Override, TextRun::NoRounding);
+    TextRun textRun(string, length, false, 0, 0, TextRun::AllowTrailingExpansion, itemStyle->direction(), isOverride(itemStyle->unicodeBidi()), TextRun::NoRounding);
     Font itemFont = style()->font();
     LayoutRect r = itemBoundingBoxRect(paintOffset, listIndex);

trunk/Source/WebCore/rendering/RenderMenuList.cpp

@@ -448 +448 @@
     
     RenderStyle* style = element->renderStyle() ? element->renderStyle() : element->computedStyle();
-    return style ? PopupMenuStyle(style->visitedDependentColor(CSSPropertyColor), itemBackgroundColor(listIndex), style->font(), style->visibility() == VISIBLE, style->display() == NONE, style->textIndent(), style->direction(), style->unicodeBidi() == Override) : menuStyle();
+    return style ? PopupMenuStyle(style->visitedDependentColor(CSSPropertyColor), itemBackgroundColor(listIndex), style->font(), style->visibility() == VISIBLE,
+        style->display() == NONE, style->textIndent(), style->direction(), isOverride(style->unicodeBidi())) : menuStyle();
 }
 
@@ -477 +478 @@
 {
     RenderStyle* s = m_innerBlock ? m_innerBlock->style() : style();
-    return PopupMenuStyle(s->visitedDependentColor(CSSPropertyColor), s->visitedDependentColor(CSSPropertyBackgroundColor), s->font(), s->visibility() == VISIBLE, s->display() == NONE, s->textIndent(), style()->direction(), style()->unicodeBidi() == Override);
+    return PopupMenuStyle(s->visitedDependentColor(CSSPropertyColor), s->visitedDependentColor(CSSPropertyBackgroundColor), s->font(), s->visibility() == VISIBLE,
+        s->display() == NONE, s->textIndent(), style()->direction(), isOverride(style()->unicodeBidi()));
 }
 

trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp

@@ -622 +622 @@
 PopupMenuStyle RenderTextControlSingleLine::menuStyle() const
 {
-    return PopupMenuStyle(style()->visitedDependentColor(CSSPropertyColor), style()->visitedDependentColor(CSSPropertyBackgroundColor), style()->font(), style()->visibility() == VISIBLE, style()->display() == NONE, style()->textIndent(), style()->direction(), style()->unicodeBidi() == Override);
+    return PopupMenuStyle(style()->visitedDependentColor(CSSPropertyColor), style()->visitedDependentColor(CSSPropertyBackgroundColor), style()->font(), style()->visibility() == VISIBLE,
+        style()->display() == NONE, style()->textIndent(), style()->direction(), isOverride(style()->unicodeBidi()));
 }
 

trunk/Source/WebCore/rendering/svg/SVGTextMetrics.cpp

@@ -76 +76 @@
                 , TextRun::AllowTrailingExpansion
                 , style->direction()
-                , style->unicodeBidi() == Override /* directionalOverride */);
+                , isOverride(style->unicodeBidi()) /* directionalOverride */);
 
     if (textRunNeedsRenderingContext(style->font()))