- Revision
- 133581
- Author
- ta...@google.com
- Date
- 2012-11-06 03:33:07 -0800 (Tue, 06 Nov 2012)
Log Message
removeAttribute('style') not working in certain circumstances
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (133580 => 133581)
--- trunk/LayoutTests/ChangeLog 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/LayoutTests/ChangeLog 2012-11-06 11:33:07 UTC (rev 133581)
@@ -1,3 +1,13 @@
+2012-11-06 Takashi Sakamoto <ta...@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 <pe...@chromium.org>
[Chromium-Android] Skip a number of crashing tests.
Added: trunk/LayoutTests/fast/css/remove-attribute-style-expected.txt (0 => 133581)
--- trunk/LayoutTests/fast/css/remove-attribute-style-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/remove-attribute-style-expected.txt 2012-11-06 11:33:07 UTC (rev 133581)
@@ -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
+
Added: trunk/LayoutTests/fast/css/remove-attribute-style.html (0 => 133581)
--- trunk/LayoutTests/fast/css/remove-attribute-style.html (rev 0)
+++ trunk/LayoutTests/fast/css/remove-attribute-style.html 2012-11-06 11:33:07 UTC (rev 133581)
@@ -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=""
+</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=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (133580 => 133581)
--- trunk/Source/WebCore/ChangeLog 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/Source/WebCore/ChangeLog 2012-11-06 11:33:07 UTC (rev 133581)
@@ -1,3 +1,42 @@
+2012-11-06 Takashi Sakamoto <ta...@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 <a...@chromium.org>
Web Inspector: dim size bar for expanded item in native memory snapshot grid
Modified: trunk/Source/WebCore/css/StylePropertySet.cpp (133580 => 133581)
--- trunk/Source/WebCore/css/StylePropertySet.cpp 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/Source/WebCore/css/StylePropertySet.cpp 2012-11-06 11:33:07 UTC (rev 133581)
@@ -978,6 +978,12 @@
CSSPropertyWidows
};
+void StylePropertySet::clear()
+{
+ ASSERT(isMutable());
+ mutablePropertyVector().clear();
+}
+
const unsigned numBlockProperties = WTF_ARRAY_LENGTH(blockProperties);
PassRefPtr<StylePropertySet> StylePropertySet::copyBlockProperties() const
Modified: trunk/Source/WebCore/css/StylePropertySet.h (133580 => 133581)
--- trunk/Source/WebCore/css/StylePropertySet.h 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/Source/WebCore/css/StylePropertySet.h 2012-11-06 11:33:07 UTC (rev 133581)
@@ -123,6 +123,7 @@
void addParsedProperties(const Vector<CSSProperty>&);
void addParsedProperty(const CSSProperty&);
+ void clear();
PassRefPtr<StylePropertySet> copyBlockProperties() const;
void removeBlockProperties();
bool removePropertiesInSet(const CSSPropertyID* set, unsigned length);
Modified: trunk/Source/WebCore/dom/Element.cpp (133580 => 133581)
--- trunk/Source/WebCore/dom/Element.cpp 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-11-06 11:33:07 UTC (rev 133581)
@@ -1662,8 +1662,11 @@
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);
}
Modified: trunk/Source/WebCore/dom/StyledElement.cpp (133580 => 133581)
--- trunk/Source/WebCore/dom/StyledElement.cpp 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/Source/WebCore/dom/StyledElement.cpp 2012-11-06 11:33:07 UTC (rev 133581)
@@ -221,6 +221,17 @@
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)
Modified: trunk/Source/WebCore/dom/StyledElement.h (133580 => 133581)
--- trunk/Source/WebCore/dom/StyledElement.h 2012-11-06 11:17:24 UTC (rev 133580)
+++ trunk/Source/WebCore/dom/StyledElement.h 2012-11-06 11:33:07 UTC (rev 133581)
@@ -48,6 +48,7 @@
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;
