Attachment 116574 Details for Bug 63903 – Ready for review but can't land until the bug 73116 is fixed

[patch] Ready for review but can't land until the bug 73116 is fixed

bug-63903-20111125001245.patch (text/plain), 14.94 KB, created by Ryosuke Niwa on 2011-11-25 00:12:46 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2011-11-25 00:12:46 PST

Size: 14.94 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 101153)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,35 @@
>+2011-11-25  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        dir=auto should imply unicode-bidi:isolate by default
>+        https://bugs.webkit.org/show_bug.cgi?id=63903
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: fast/css/default-bidi-css-rules.html
>+
>+        Fixed WebKit's implementation of bdo, bdi, and output elements to match HTML5 spec section 10.3.5:
>+        http://dev.w3.org/html5/spec/Overview.html#bidirectional-text
>+
>+        Any element other than bdo, textarea, and pre with dir=auto should use unicode-bidi: -webkit-isolate
>+        by default.
>+
>+        * css/html.css:
>+        (bdi, output): bdi and output should both use -webkit-isolate as the default value for unicode-bidi.
>+        (bdo): bdo should use -webkit-override as the default value for unicode-bidi.
>+        * dom/MappedAttributeEntry.h: Add eBDI, which is used by bdi and output elements.
>+        * html/HTMLElement.cpp:
>+        (WebCore::HTMLElement::mapToEntry): Don't share the cache for dir attribute between bdi and other elements
>+        just like we don't share the cache for dir attribute between bdo and other elements.
>+        (WebCore::unicodeBidiAttributeForDirAuto): Don't set bidi-override for bdo element since this is done
>+        in the UA stylesheet now. Also return -webkit-isolate as the default because now that this function is
>+        called when dir=auto as the name implies.
>+        (WebCore::HTMLElement::parseMappedAttribute): Don't call unicodeBidiAttributeForDirAuto when dir is not
>+        auto. Also set unicode-bidi to embed when dir is not auto and the element is neither bdi, bdo, nor output.
>+        * html/HTMLOutputElement.cpp:
>+        (WebCore::HTMLOutputElement::mapToEntry): Don't share the cache for dir attribute between output and other elements.
>+        * html/HTMLOutputElement.h:
>+        * html/HTMLTagNames.in:
>+
> 2011-11-24  Kentaro Hara  <haraken@chromium.org>
> 
>         Initialize global variables during IDLParser object creation
>Index: Source/WebCore/css/html.css
>===================================================================
>--- Source/WebCore/css/html.css	(revision 101118)
>+++ Source/WebCore/css/html.css	(working copy)
>@@ -992,10 +992,14 @@ summary::-webkit-details-marker {
>     margin-right: 0.4em;
> }
> 
>-bdi {
>+bdi, output {
>     unicode-bidi: -webkit-isolate;
> }
> 
>+bdo {
>+    unicode-bidi: bidi-override;
>+}
>+
> /* page */
> 
> @page {
>Index: Source/WebCore/dom/MappedAttributeEntry.h
>===================================================================
>--- Source/WebCore/dom/MappedAttributeEntry.h	(revision 101118)
>+++ Source/WebCore/dom/MappedAttributeEntry.h	(working copy)
>@@ -40,6 +40,7 @@ enum MappedAttributeEntry {
>     , eTable
>     , eCell
>     , eCaption
>+    , eBDI
>     , eBDO
>     , ePre
> #if ENABLE(SVG)
>Index: Source/WebCore/html/HTMLElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLElement.cpp	(revision 101118)
>+++ Source/WebCore/html/HTMLElement.cpp	(working copy)
>@@ -127,7 +127,12 @@ bool HTMLElement::mapToEntry(const Quali
>         return false;
>     }
>     if (attrName == dirAttr) {
>-        result = hasLocalName(bdoTag) ? eBDO : eUniversal;
>+        if (hasLocalName(bdoTag))
>+            result = eBDO;
>+        else if (hasLocalName(bdiTag))
>+            result = eBDI;
>+        else
>+            result = eUniversal;
>         return true;
>     }
> 
>@@ -136,11 +141,9 @@ bool HTMLElement::mapToEntry(const Quali
> 
> static inline int unicodeBidiAttributeForDirAuto(HTMLElement* element)
> {
>-    if (element->hasLocalName(bdoTag))
>-        return CSSValueBidiOverride;
>     if (element->hasLocalName(preTag) || element->hasLocalName(textareaTag))
>         return CSSValueWebkitPlaintext;
>-    return CSSValueEmbed;
>+    return CSSValueWebkitIsolate;
> }
> 
> static unsigned parseBorderWidthAttribute(Attribute* attr)
>@@ -190,10 +193,14 @@ void HTMLElement::parseMappedAttribute(A
>     } else if (attr->name() == langAttr) {
>         // FIXME: Implement
>     } else if (attr->name() == dirAttr) {
>-        if (!equalIgnoringCase(attr->value(), "auto"))
>+        bool dirIsAuto = equalIgnoringCase(attr->value(), "auto");
>+        if (!dirIsAuto)
>             addCSSProperty(attr, CSSPropertyDirection, attr->value());
>         dirAttributeChanged(attr);
>-        addCSSProperty(attr, CSSPropertyUnicodeBidi, unicodeBidiAttributeForDirAuto(this));
>+        if (dirIsAuto)
>+            addCSSProperty(attr, CSSPropertyUnicodeBidi, unicodeBidiAttributeForDirAuto(this));
>+        else if (!hasTagName(bdiTag) && !hasTagName(bdoTag) && !hasTagName(outputTag))
>+            addCSSProperty(attr, CSSPropertyUnicodeBidi, CSSValueEmbed);
>     } else if (attr->name() == draggableAttr) {
>         const AtomicString& value = attr->value();
>         if (equalIgnoringCase(value, "true")) {
>Index: Source/WebCore/html/HTMLOutputElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLOutputElement.cpp	(revision 101118)
>+++ Source/WebCore/html/HTMLOutputElement.cpp	(working copy)
>@@ -61,6 +61,14 @@ bool HTMLOutputElement::supportsFocus() 
>     return Node::supportsFocus() && !disabled();
> }
> 
>+bool HTMLOutputElement::mapToEntry(const QualifiedName& attrName, MappedAttributeEntry& result) const
>+{
>+    if (attrName == HTMLNames::dirAttr)
>+        return eBDO;
>+
>+    return HTMLElement::mapToEntry(attrName, result);
>+}
>+
> void HTMLOutputElement::parseMappedAttribute(Attribute* attr)
> {
>     if (attr->name() == HTMLNames::forAttr)
>Index: Source/WebCore/html/HTMLOutputElement.h
>===================================================================
>--- Source/WebCore/html/HTMLOutputElement.h	(revision 101118)
>+++ Source/WebCore/html/HTMLOutputElement.h	(working copy)
>@@ -55,6 +55,7 @@ public:
> private:
>     HTMLOutputElement(const QualifiedName&, Document*, HTMLFormElement*);
> 
>+    virtual bool mapToEntry(const QualifiedName& attrName, MappedAttributeEntry& result) const OVERRIDE;
>     virtual void parseMappedAttribute(Attribute*);
>     virtual const AtomicString& formControlType() const;
>     virtual bool isEnumeratable() const { return true; }
>Index: Source/WebCore/html/HTMLTagNames.in
>===================================================================
>--- Source/WebCore/html/HTMLTagNames.in	(revision 101118)
>+++ Source/WebCore/html/HTMLTagNames.in	(working copy)
>@@ -15,6 +15,7 @@ audio wrapperOnlyIfMediaIsAvailable, con
> b interfaceName=HTMLElement
> base
> basefont interfaceName=HTMLBaseFontElement
>+bdi interfaceName=HTMLElement
> bdo interfaceName=HTMLElement
> bgsound interfaceName=HTMLElement
> big interfaceName=HTMLElement
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 101153)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2011-11-24  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        dir=auto should imply unicode-bidi:isolate by default
>+        https://bugs.webkit.org/show_bug.cgi?id=63903
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a test to ensure WebKit implements HTML5 bidirectional text support properly:
>+        http://dev.w3.org/html5/spec/Overview.html#bidirectional-text
>+
>+        * fast/css/default-bidi-css-rules-expected.txt: Added.
>+        * fast/css/default-bidi-css-rules.html: Added.
>+
> 2011-11-24  Arko Saha  <arko@motorola.com>
> 
>         Microdata: Support for properties attribute.
>Index: LayoutTests/fast/css/default-bidi-css-rules-expected.txt
>===================================================================
>--- LayoutTests/fast/css/default-bidi-css-rules-expected.txt	(revision 0)
>+++ LayoutTests/fast/css/default-bidi-css-rules-expected.txt	(revision 0)
>@@ -0,0 +1,76 @@
>+This test checks the default rules for direction and unicode-bidi CSS properties.
>+
>+PASS styleOf("div", {}).direction is "ltr"
>+PASS styleOf("div", {}).unicodeBidi is "normal"
>+PASS styleOf("div", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("div", {"dir":"ltr"}).unicodeBidi is "embed"
>+PASS styleOf("div", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("div", {"dir":"rtl"}).unicodeBidi is "embed"
>+PASS styleOf("div", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("div", {"dir":"auto"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("div", {"dir":""}).direction is "ltr"
>+PASS styleOf("div", {"dir":""}).unicodeBidi is "embed"
>+PASS styleOf("span", {}).direction is "ltr"
>+PASS styleOf("span", {}).unicodeBidi is "normal"
>+PASS styleOf("span", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("span", {"dir":"ltr"}).unicodeBidi is "embed"
>+PASS styleOf("span", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("span", {"dir":"rtl"}).unicodeBidi is "embed"
>+PASS styleOf("span", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("span", {"dir":"auto"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("span", {"dir":""}).direction is "ltr"
>+PASS styleOf("span", {"dir":""}).unicodeBidi is "embed"
>+PASS styleOf("bdi", {}).direction is "ltr"
>+PASS styleOf("bdi", {}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("bdi", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("bdi", {"dir":"ltr"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("bdi", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("bdi", {"dir":"rtl"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("bdi", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("bdi", {"dir":"auto"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("bdi", {"dir":""}).direction is "ltr"
>+PASS styleOf("bdi", {"dir":""}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("output", {}).direction is "ltr"
>+PASS styleOf("output", {}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("output", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("output", {"dir":"ltr"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("output", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("output", {"dir":"rtl"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("output", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("output", {"dir":"auto"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("output", {"dir":""}).direction is "ltr"
>+PASS styleOf("output", {"dir":""}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("bdo", {}).direction is "ltr"
>+PASS styleOf("bdo", {}).unicodeBidi is "bidi-override"
>+PASS styleOf("bdo", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("bdo", {"dir":"ltr"}).unicodeBidi is "bidi-override"
>+PASS styleOf("bdo", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("bdo", {"dir":"rtl"}).unicodeBidi is "bidi-override"
>+PASS styleOf("bdo", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("bdo", {"dir":"auto"}).unicodeBidi is "-webkit-isolate"
>+PASS styleOf("bdo", {"dir":""}).direction is "ltr"
>+PASS styleOf("bdo", {"dir":""}).unicodeBidi is "bidi-override"
>+PASS styleOf("textarea", {}).direction is "ltr"
>+PASS styleOf("textarea", {}).unicodeBidi is "normal"
>+PASS styleOf("textarea", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("textarea", {"dir":"ltr"}).unicodeBidi is "embed"
>+PASS styleOf("textarea", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("textarea", {"dir":"rtl"}).unicodeBidi is "embed"
>+PASS styleOf("textarea", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("textarea", {"dir":"auto"}).unicodeBidi is "-webkit-plaintext"
>+PASS styleOf("textarea", {"dir":""}).direction is "ltr"
>+PASS styleOf("textarea", {"dir":""}).unicodeBidi is "embed"
>+PASS styleOf("pre", {}).direction is "ltr"
>+PASS styleOf("pre", {}).unicodeBidi is "normal"
>+PASS styleOf("pre", {"dir":"ltr"}).direction is "ltr"
>+PASS styleOf("pre", {"dir":"ltr"}).unicodeBidi is "embed"
>+PASS styleOf("pre", {"dir":"rtl"}).direction is "rtl"
>+PASS styleOf("pre", {"dir":"rtl"}).unicodeBidi is "embed"
>+PASS styleOf("pre", {"dir":"auto"}).direction is "ltr"
>+PASS styleOf("pre", {"dir":"auto"}).unicodeBidi is "-webkit-plaintext"
>+PASS styleOf("pre", {"dir":""}).direction is "ltr"
>+PASS styleOf("pre", {"dir":""}).unicodeBidi is "embed"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>Index: LayoutTests/fast/css/default-bidi-css-rules.html
>===================================================================
>--- LayoutTests/fast/css/default-bidi-css-rules.html	(revision 0)
>+++ LayoutTests/fast/css/default-bidi-css-rules.html	(revision 0)
>@@ -0,0 +1,74 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="../js/resources/js-test-pre.js"></script>
>+<p>This test checks <a href="http://dev.w3.org/html5/spec/Overview.html#bidirectional-text">the default rules for direction and unicode-bidi CSS properties</a>.</p>
>+<div id="container"></div>
>+<div id="console"></div>
>+<script>
>+
>+var container = document.getElementById('container');
>+
>+function styleOf(name, attributes) {
>+    var element = document.createElement(name);
>+    for (var name in attributes) {
>+        var value = attributes[name];
>+        element.setAttribute(name, value);
>+    }
>+    container.appendChild(element);
>+    return getComputedStyle(element);
>+}
>+
>+var tests = [
>+    ['div', {}, 'ltr', 'normal'],
>+    ['div', {'dir': 'ltr'}, 'ltr', 'embed'],
>+    ['div', {'dir': 'rtl'}, 'rtl', 'embed'],
>+    ['div', {'dir': 'auto'}, 'ltr', '-webkit-isolate'],
>+    ['div', {'dir': ''}, 'ltr', 'embed'],
>+
>+    ['span', {}, 'ltr', 'normal'],
>+    ['span', {'dir': 'ltr'}, 'ltr', 'embed'],
>+    ['span', {'dir': 'rtl'}, 'rtl', 'embed'],
>+    ['span', {'dir': 'auto'}, 'ltr', '-webkit-isolate'],
>+    ['span', {'dir': ''}, 'ltr', 'embed'],
>+
>+    ['bdi', {}, 'ltr', '-webkit-isolate'],
>+    ['bdi', {'dir': 'ltr'}, 'ltr', '-webkit-isolate'],
>+    ['bdi', {'dir': 'rtl'}, 'rtl', '-webkit-isolate'],
>+    ['bdi', {'dir': 'auto'}, 'ltr', '-webkit-isolate'],
>+    ['bdi', {'dir': ''}, 'ltr', '-webkit-isolate'],
>+
>+    ['output', {}, 'ltr', '-webkit-isolate'],
>+    ['output', {'dir': 'ltr'}, 'ltr', '-webkit-isolate'],
>+    ['output', {'dir': 'rtl'}, 'rtl', '-webkit-isolate'],
>+    ['output', {'dir': 'auto'}, 'ltr', '-webkit-isolate'],
>+    ['output', {'dir': ''}, 'ltr', '-webkit-isolate'],
>+
>+    ['bdo', {}, 'ltr', 'bidi-override'],
>+    ['bdo', {'dir': 'ltr'}, 'ltr', 'bidi-override'],
>+    ['bdo', {'dir': 'rtl'}, 'rtl', 'bidi-override'],
>+    ['bdo', {'dir': 'auto'}, 'ltr', '-webkit-isolate'],
>+    ['bdo', {'dir': ''}, 'ltr', 'bidi-override'],
>+
>+    ['textarea', {}, 'ltr', 'normal'],
>+    ['textarea', {'dir': 'ltr'}, 'ltr', 'embed'],
>+    ['textarea', {'dir': 'rtl'}, 'rtl', 'embed'],
>+    ['textarea', {'dir': 'auto'}, 'ltr', '-webkit-plaintext'],
>+    ['textarea', {'dir': ''}, 'ltr', 'embed'],
>+
>+    ['pre', {}, 'ltr', 'normal'],
>+    ['pre', {'dir': 'ltr'}, 'ltr', 'embed'],
>+    ['pre', {'dir': 'rtl'}, 'rtl', 'embed'],
>+    ['pre', {'dir': 'auto'}, 'ltr', '-webkit-plaintext'],
>+    ['pre', {'dir': ''}, 'ltr', 'embed'],
>+].forEach(function (test) {
>+    shouldBe('styleOf("' + test[0] + '", ' + JSON.stringify(test[1]) + ').direction', '"' + test[2] + '"');
>+    container.innerHTML = '';
>+    shouldBe('styleOf("' + test[0] + '", ' + JSON.stringify(test[1]) + ').unicodeBidi', '"' + test[3] + '"');
>+    container.innerHTML = '';
>+});
>+
>+</script>
>+<script src="../js/resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 63903: 116470 | 116479 | 116574 | 116667