removeAttribute('style') not working in certain circumstances
authortasak@google.com <tasak@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Nov 2012 11:33:07 +0000 (11:33 +0000)
committertasak@google.com <tasak@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Nov 2012 11:33:07 +0000 (11:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=99295

Reviewed by Ryosuke Niwa.

Source/WebCore:

After web developers did style.XXXX=YYYY for some element, the inline
style should be always removable by using "removeAttribute('style')".
Currently it depends on whether web developers invokes
getAttribute('style'), setAttribute('style), and so on. E.g. once they
invoke getAttribute('style'), removeAttribute('style') works. This is
very confusing behavior.
Looking at Firefox browser, removeAttribute('style') always removes
all inline styles.

Test: fast/css/remove-attribute-style.html

* css/StylePropertySet.cpp:
(WebCore::StylePropertySet::clear):
Added a new method to remove all style properties.
(WebCore):
* css/StylePropertySet.h:
(StylePropertySet):
* dom/Element.cpp:
(WebCore::Element::removeAttribute):
If 'style' is given but the element has no style attribute, the old
code did nothing. However, if the element is styled element and has any
inline styles, the inline styles should be removed. So invoke
StyledElement::removeAllInlineStyleProperties and if any inline styles
are removed, invoke style recalc, too.
* dom/StyledElement.cpp:
(WebCore::StyledElement::removeAllInlineStyleProperties):
Added a new method to remove all inline style propeties. If any inline
style is removed, invoke inlineStyleChanged() to force style recalc.
(WebCore):
* dom/StyledElement.h:
(StyledElement):

LayoutTests:

* fast/css/remove-attribute-style-expected.txt: Added.
* fast/css/remove-attribute-style.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@133581 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/remove-attribute-style-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/remove-attribute-style.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/StylePropertySet.cpp
Source/WebCore/css/StylePropertySet.h
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/dom/StyledElement.h

index 6c7efe1..336bd14 100644 (file)
@@ -1,3 +1,13 @@
+2012-11-06  Takashi Sakamoto  <tasak@google.com>
+
+        removeAttribute('style') not working in certain circumstances
+        https://bugs.webkit.org/show_bug.cgi?id=99295
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/css/remove-attribute-style-expected.txt: Added.
+        * fast/css/remove-attribute-style.html: Added.
+
 2012-11-06  Peter Beverloo  <peter@chromium.org>
 
         [Chromium-Android] Skip a number of crashing tests.
diff --git a/LayoutTests/fast/css/remove-attribute-style-expected.txt b/LayoutTests/fast/css/remove-attribute-style-expected.txt
new file mode 100644 (file)
index 0000000..9816fae
--- /dev/null
@@ -0,0 +1,17 @@
+[bug 99295] removeAttribute('style') not working in certain circumstances. If this test passes, all backgroundColors are rgba(0, 0, 0, 0), i.e. all styles are removed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS elementWithoutStyleAttribute.style.backgroundColor = 'red'; elementWithoutStyleAttribute.removeAttribute('style'); getComputedStyle(elementWithoutStyleAttribute).backgroundColor is "rgba(0, 0, 0, 0)"
+PASS getComputedStyle(elementWithEmptyStyleAttribute).backgroundColor is "rgba(0, 0, 0, 0)"
+PASS getComputedStyle(elementWithStyleAttribute).backgroundColor is "rgba(0, 0, 0, 0)"
+PASS elementWithoutStyleAttribute2.getAttribute('style') is null
+PASS elementWithoutStyleAttribute2.style.backgroundColor = 'red'; elementWithoutStyleAttribute2.removeAttribute('style'); getComputedStyle(elementWithoutStyleAttribute2).backgroundColor is "rgba(0, 0, 0, 0)"
+PASS elementWithoutStyleAttribute3.getAttribute('style') is "background-color: red;"
+PASS getComputedStyle(elementWithoutStyleAttribute3).backgroundColor is "rgba(0, 0, 0, 0)"
+PASS elementWithoutStyleAttribute4.style.backgroundColor = 'red'; elementWithoutStyleAttribute4.setAttribute('style', ''); elementWithoutStyleAttribute4.removeAttribute('style'); getComputedStyle(elementWithoutStyleAttribute4).backgroundColor is "rgba(0, 0, 0, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/css/remove-attribute-style.html b/LayoutTests/fast/css/remove-attribute-style.html
new file mode 100644 (file)
index 0000000..a083d34
--- /dev/null
@@ -0,0 +1,68 @@
+<!doctype html>
+<html>
+<head>
+<style type="text/css">
+td {
+    display: table-cell;
+    width: 200px;
+    height: 80px;
+    border: 1px solid #ccc;
+    text-align: center;
+    vertical-align: middle;
+}
+</style>
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+  <table id="table">
+    <tr>
+      <td id="elementWithoutStyleAttribute">no HTML style attribute, no get/setAttribute</td>
+      <td id="elementWithEmptyStyleAttribute" style="">empty HTML style attribute, no get/setAttribute</td>
+      <td id="elementWithStyleAttribute" style="opacity: 1;">non-empty HTML style attribute, no get/setAttribute</td>
+    </tr>
+    <tr>
+      <td id="elementWithoutStyleAttribute2">no HTML style attribute, getAttribute before modifying IDL attribute</td>
+      <td id="elementWithoutStyleAttribute3">no HTML style attribute, getAttribute after modifying IDL attribute</td>
+      <td id="elementWithoutStyleAttribute4">no HTML style attribute, setAttribute before removeAttribute</td>
+    </tr>
+  </table>
+  <div id="console"></div>
+  <script>
+    description("[bug 99295] removeAttribute('style') not working in certain circumstances. If this test passes, all backgroundColors are rgba(0, 0, 0, 0), i.e. all styles are removed.");
+
+    var elementWithoutStyleAttribute = $('elementWithoutStyleAttribute'),
+        elementWithEmptyStyleAttribute = $('elementWithEmptyStyleAttribute'),
+        elementWithStyleAttribute = $('elementWithStyleAttribute'),
+        elementWithoutStyleAttribute2 = $('elementWithoutStyleAttribute2'),
+        elementWithoutStyleAttribute3 = $('elementWithoutStyleAttribute3'),
+        elementWithoutStyleAttribute4 = $('elementWithoutStyleAttribute4');
+
+    shouldBe("elementWithoutStyleAttribute.style.backgroundColor = 'red'; elementWithoutStyleAttribute.removeAttribute('style'); getComputedStyle(elementWithoutStyleAttribute).backgroundColor", '"rgba(0, 0, 0, 0)"');
+
+    elementWithEmptyStyleAttribute.style.backgroundColor = 'red';
+    elementWithEmptyStyleAttribute.removeAttribute('style');
+    shouldBe("getComputedStyle(elementWithEmptyStyleAttribute).backgroundColor", '"rgba(0, 0, 0, 0)"');
+
+    elementWithStyleAttribute.style.backgroundColor = 'red';
+    elementWithStyleAttribute.removeAttribute('style');
+    shouldBe("getComputedStyle(elementWithStyleAttribute).backgroundColor", '"rgba(0, 0, 0, 0)"');
+
+    shouldBeNull("elementWithoutStyleAttribute2.getAttribute('style')");
+    shouldBe("elementWithoutStyleAttribute2.style.backgroundColor = 'red'; elementWithoutStyleAttribute2.removeAttribute('style'); getComputedStyle(elementWithoutStyleAttribute2).backgroundColor", '"rgba(0, 0, 0, 0)"');
+
+    elementWithoutStyleAttribute3.style.backgroundColor = 'red';
+    shouldBe("elementWithoutStyleAttribute3.getAttribute('style')", '"background-color: red;"');
+    elementWithoutStyleAttribute3.removeAttribute('style');
+    shouldBe("getComputedStyle(elementWithoutStyleAttribute3).backgroundColor", '"rgba(0, 0, 0, 0)"');
+
+    shouldBe("elementWithoutStyleAttribute4.style.backgroundColor = 'red'; elementWithoutStyleAttribute4.setAttribute('style', ''); elementWithoutStyleAttribute4.removeAttribute('style'); getComputedStyle(elementWithoutStyleAttribute4).backgroundColor", '"rgba(0, 0, 0, 0)"');
+
+    function $(id) {
+        return document.getElementById(id);
+    }
+
+    document.getElementById('table').innerHTML = '';
+  </script>
+  <script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 0fdddb1..8a267c2 100644 (file)
@@ -1,3 +1,42 @@
+2012-11-06  Takashi Sakamoto  <tasak@google.com>
+
+        removeAttribute('style') not working in certain circumstances
+        https://bugs.webkit.org/show_bug.cgi?id=99295
+
+        Reviewed by Ryosuke Niwa.
+
+        After web developers did style.XXXX=YYYY for some element, the inline
+        style should be always removable by using "removeAttribute('style')".
+        Currently it depends on whether web developers invokes
+        getAttribute('style'), setAttribute('style), and so on. E.g. once they
+        invoke getAttribute('style'), removeAttribute('style') works. This is
+        very confusing behavior.
+        Looking at Firefox browser, removeAttribute('style') always removes
+        all inline styles.
+
+        Test: fast/css/remove-attribute-style.html
+
+        * css/StylePropertySet.cpp:
+        (WebCore::StylePropertySet::clear):
+        Added a new method to remove all style properties.
+        (WebCore):
+        * css/StylePropertySet.h:
+        (StylePropertySet):
+        * dom/Element.cpp:
+        (WebCore::Element::removeAttribute):
+        If 'style' is given but the element has no style attribute, the old
+        code did nothing. However, if the element is styled element and has any
+        inline styles, the inline styles should be removed. So invoke
+        StyledElement::removeAllInlineStyleProperties and if any inline styles
+        are removed, invoke style recalc, too.
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::removeAllInlineStyleProperties):
+        Added a new method to remove all inline style propeties. If any inline
+        style is removed, invoke inlineStyleChanged() to force style recalc.
+        (WebCore):
+        * dom/StyledElement.h:
+        (StyledElement):
+
 2012-11-06  Alexei Filippov  <alph@chromium.org>
 
         Web Inspector: dim size bar for expanded item in native memory snapshot grid
index fe22c77..110b802 100644 (file)
@@ -978,6 +978,12 @@ static const CSSPropertyID blockProperties[] = {
     CSSPropertyWidows
 };
 
+void StylePropertySet::clear()
+{
+    ASSERT(isMutable());
+    mutablePropertyVector().clear();
+}
+
 const unsigned numBlockProperties = WTF_ARRAY_LENGTH(blockProperties);
 
 PassRefPtr<StylePropertySet> StylePropertySet::copyBlockProperties() const
index 435dc4d..f70f1ff 100644 (file)
@@ -123,6 +123,7 @@ public:
     void addParsedProperties(const Vector<CSSProperty>&);
     void addParsedProperty(const CSSProperty&);
 
+    void clear();
     PassRefPtr<StylePropertySet> copyBlockProperties() const;
     void removeBlockProperties();
     bool removePropertiesInSet(const CSSPropertyID* set, unsigned length);
index d3c1050..26672ec 100644 (file)
@@ -1662,8 +1662,11 @@ void Element::removeAttribute(const AtomicString& name)
 
     AtomicString localName = shouldIgnoreAttributeCase(this) ? name.lower() : name;
     size_t index = attributeData()->getAttributeItemIndex(localName, false);
-    if (index == notFound)
+    if (index == notFound) {
+        if (UNLIKELY(localName == styleAttr) && !isStyleAttributeValid() && isStyledElement())
+            static_cast<StyledElement*>(this)->removeAllInlineStyleProperties();
         return;
+    }
 
     removeAttributeInternal(index, NotInSynchronizationOfLazyAttribute);
 }
index 576aa22..c4c86ec 100644 (file)
@@ -221,6 +221,17 @@ bool StyledElement::removeInlineStyleProperty(CSSPropertyID propertyID)
     return changes;
 }
 
+void StyledElement::removeAllInlineStyleProperties()
+{
+    if (!attributeData() || !attributeData()->inlineStyle())
+        return;
+    StylePropertySet* inlineStylePropertySet = ensureInlineStyle();
+    if (inlineStylePropertySet->isEmpty())
+        return;
+    inlineStylePropertySet->clear();
+    inlineStyleChanged();
+}
+
 void StyledElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const
 {
     if (const StylePropertySet* inlineStyle = attributeData() ? attributeData()->inlineStyle() : 0)
index cf59ed1..a5d3083 100644 (file)
@@ -48,6 +48,7 @@ public:
     bool setInlineStyleProperty(CSSPropertyID, double value, CSSPrimitiveValue::UnitTypes, bool important = false);
     bool setInlineStyleProperty(CSSPropertyID, const String& value, bool important = false);
     bool removeInlineStyleProperty(CSSPropertyID);
+    void removeAllInlineStyleProperties();
     
     virtual CSSStyleDeclaration* style() OVERRIDE;