Attachment 159948 Details for Bug 93665 – Patch

[patch] Patch

bug-93665-20120822115543.patch (text/plain), 13.72 KB, created by Erik Arvidsson on 2012-08-22 08:55:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Erik Arvidsson

Created: 2012-08-22 08:55:45 PDT

Size: 13.72 KB

patch

obsolete

>Subversion Revision: 125796
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 203746f31fa887e07f2e456f03fa251b251b9143..e6b6515d1bb953dd69bc668078b66897c93b9e26 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2012-08-22  Erik Arvidsson  <arv@chromium.org>
>+
>+        Class attribute logic needs to be moved to Element (from StyledElement)
>+        https://bugs.webkit.org/show_bug.cgi?id=93665
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Before this change classAttributeChanged was only called for StyledElement. With this refactoring
>+        it gets called for all Elements when the class attribute changes.
>+
>+        Test: fast/dom/Element/class-list-update.html
>+
>+        * css/StyleResolver.cpp:
>+        (WebCore::StyleResolver::collectMatchingRules): classNames is now part of Element.
>+        * dom/ClassNodeList.cpp:
>+        (WebCore::ClassNodeList::nodeMatches): To match the old behavior we only include StyledElements.
>+        * dom/Element.cpp:
>+        (WebCore::Element::attributeChanged): Moved the call to parseAttribute here, from StyledElement::attributeChanged.
>+        (WebCore::Element::parseAttribute): Moved from StyledElement.
>+        (WebCore):
>+        (WebCore::Element::classAttributeChanged): Ditto.
>+        * dom/Element.h:
>+        (Element):
>+        (WebCore::Element::classNames): Ditto.
>+        (WebCore):
>+        * dom/StyledElement.cpp:
>+        (WebCore::StyledElement::attributeChanged): Let Element::attributeChanged call parseAttribtue instead.
>+        (WebCore::StyledElement::parseAttribute): Move the class attribute handling to Element::parseAttribute.
>+        * dom/StyledElement.h:
>+        (StyledElement):
>+
> 2012-08-15  Levi Weintraub  <leviw@chromium.org>
> 
>         Accumulate sub-pixel offsets through layers and transforms
>diff --git a/Source/WebCore/css/StyleResolver.cpp b/Source/WebCore/css/StyleResolver.cpp
>index 2a6b049a77ce3e0bafdd4163056f9f2fb3877ba3..b4f321370886bf50196c5eb4244bbc711514f24c 100644
>--- a/Source/WebCore/css/StyleResolver.cpp
>+++ b/Source/WebCore/css/StyleResolver.cpp
>@@ -886,9 +886,8 @@ void StyleResolver::collectMatchingRules(RuleSet* rules, int& firstRuleIndex, in
>     if (m_element->hasID())
>         collectMatchingRulesForList(rules->idRules(m_element->idForStyleResolution().impl()), firstRuleIndex, lastRuleIndex, options);
>     if (m_element->hasClass()) {
>-        ASSERT(m_styledElement);
>-        for (size_t i = 0; i < m_styledElement->classNames().size(); ++i)
>-            collectMatchingRulesForList(rules->classRules(m_styledElement->classNames()[i].impl()), firstRuleIndex, lastRuleIndex, options);
>+        for (size_t i = 0; i < m_element->classNames().size(); ++i)
>+            collectMatchingRulesForList(rules->classRules(m_element->classNames()[i].impl()), firstRuleIndex, lastRuleIndex, options);
>     }
>     const AtomicString& pseudoId = m_element->shadowPseudoId();
>     if (!pseudoId.isEmpty()) {
>diff --git a/Source/WebCore/dom/ClassNodeList.cpp b/Source/WebCore/dom/ClassNodeList.cpp
>index c0ce6d2e040b7a492918a518a64a6d6ede86fda4..54096633fc7af29b069a9ec807c94d366d2e4b9c 100644
>--- a/Source/WebCore/dom/ClassNodeList.cpp
>+++ b/Source/WebCore/dom/ClassNodeList.cpp
>@@ -54,8 +54,11 @@ bool ClassNodeList::nodeMatches(Element* testNode) const
>         return false;
>     if (!m_classNames.size())
>         return false;
>-    ASSERT(testNode->isStyledElement());
>-    return static_cast<StyledElement*>(testNode)->classNames().containsAll(m_classNames);
>+    // TODO: DOM4 allows getElementsByClassName to return non StyledElement.
>+    // https://bugs.webkit.org/show_bug.cgi?id=94718
>+    if (!testNode->isStyledElement())
>+        return false;
>+    return testNode->classNames().containsAll(m_classNames);
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index 3c1b4b7a40380f76582124ca7b33256b75e5cfa1..77ce791c9fe7eaa89660947efed2de5ad5ad4ff7 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -695,6 +695,8 @@ inline void Element::setAttributeInternal(size_t index, const QualifiedName& nam
> 
> void Element::attributeChanged(const Attribute& attribute)
> {
>+    parseAttribute(attribute);
>+
>     document()->incDOMTreeVersion();
> 
>     if (isIdAttributeName(attribute.name())) {
>@@ -746,6 +748,34 @@ void Element::attributeChanged(const Attribute& attribute)
>         document()->axObjectCache()->postNotification(this, AXObjectCache::AXInvalidStatusChanged, true);
> }
> 
>+void Element::parseAttribute(const Attribute& attribute)
>+{
>+    if (attribute.name() == classAttr)
>+        classAttributeChanged(attribute.value());
>+}
>+
>+void Element::classAttributeChanged(const AtomicString& newClassString)
>+{
>+    const UChar* characters = newClassString.characters();
>+    unsigned length = newClassString.length();
>+    unsigned i;
>+    for (i = 0; i < length; ++i) {
>+        if (isNotHTMLSpace(characters[i]))
>+            break;
>+    }
>+    bool hasClass = i < length;
>+    if (hasClass) {
>+        const bool shouldFoldCase = document()->inQuirksMode();
>+        ensureAttributeData()->setClass(newClassString, shouldFoldCase);
>+    } else if (attributeData())
>+        mutableAttributeData()->clearClass();
>+
>+    if (DOMTokenList* classList = optionalClassList())
>+        static_cast<ClassList*>(classList)->reset(newClassString);
>+
>+    setNeedsStyleRecalc();
>+}
>+
> // Returns true is the given attribute is an event handler.
> // We consider an event handler any attribute that begins with "on".
> // It is a simple solution that has the advantage of not requiring any
>diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h
>index d056a9d435deb40e82425466991c8b0f7c00a3e7..24dc4ac41350f47e8343d12342e46e28e90ea6ea 100644
>--- a/Source/WebCore/dom/Element.h
>+++ b/Source/WebCore/dom/Element.h
>@@ -246,6 +246,7 @@ public:
> 
>     // This method is called whenever an attribute is added, changed or removed.
>     virtual void attributeChanged(const Attribute&);
>+    virtual void parseAttribute(const Attribute&);
> 
>     // Only called by the parser immediately after element construction.
>     void parserSetAttributes(const Vector<Attribute>&, FragmentScriptingPermission);
>@@ -428,6 +429,7 @@ public:
> 
>     bool hasID() const;
>     bool hasClass() const;
>+    const SpaceSplitString& classNames() const;
> 
>     IntSize savedLayerScrollOffset() const;
>     void setSavedLayerScrollOffset(const IntSize&);
>@@ -469,6 +471,11 @@ protected:
>     PassRefPtr<HTMLCollection> ensureCachedHTMLCollection(CollectionType);
>     HTMLCollection* cachedHTMLCollection(CollectionType);
> 
>+    // classAttributeChanged() exists to share code between
>+    // parseAttribute (called via setAttribute()) and
>+    // svgAttributeChanged (called when element.className.baseValue is set)
>+    void classAttributeChanged(const AtomicString& newClassString);
>+
> private:
>     void updateInvalidAttributes() const;
> 
>@@ -695,6 +702,13 @@ inline void Element::setIdAttribute(const AtomicString& value)
>     setAttribute(document()->idAttributeName(), value);
> }
> 
>+inline const SpaceSplitString& Element::classNames() const
>+{
>+    ASSERT(hasClass());
>+    ASSERT(attributeData());
>+    return attributeData()->classNames();
>+}
>+
> inline size_t Element::attributeCount() const
> {
>     ASSERT(attributeData());
>diff --git a/Source/WebCore/dom/StyledElement.cpp b/Source/WebCore/dom/StyledElement.cpp
>index c5616b082289d9a696ff90a3519fe2b7a9463c29..e75cd531d16f8d8e2023692796f1fe49ac8f7522 100644
>--- a/Source/WebCore/dom/StyledElement.cpp
>+++ b/Source/WebCore/dom/StyledElement.cpp
>@@ -147,8 +147,6 @@ CSSStyleDeclaration* StyledElement::style()
> 
> void StyledElement::attributeChanged(const Attribute& attribute)
> {
>-    parseAttribute(attribute);
>-
>     if (isPresentationAttribute(attribute.name())) {
>         setAttributeStyleDirty();
>         setNeedsStyleRecalc(InlineStyleChange);
>@@ -157,28 +155,6 @@ void StyledElement::attributeChanged(const Attribute& attribute)
>     Element::attributeChanged(attribute);
> }
> 
>-void StyledElement::classAttributeChanged(const AtomicString& newClassString)
>-{
>-    const UChar* characters = newClassString.characters();
>-    unsigned length = newClassString.length();
>-    unsigned i;
>-    for (i = 0; i < length; ++i) {
>-        if (isNotHTMLSpace(characters[i]))
>-            break;
>-    }
>-    bool hasClass = i < length;
>-    if (hasClass) {
>-        const bool shouldFoldCase = document()->inQuirksMode();
>-        ensureAttributeData()->setClass(newClassString, shouldFoldCase);
>-    } else if (attributeData())
>-        mutableAttributeData()->clearClass();
>-
>-    if (DOMTokenList* classList = optionalClassList())
>-        static_cast<ClassList*>(classList)->reset(newClassString);
>-
>-    setNeedsStyleRecalc();
>-}
>-
> void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, ShouldReparseStyleAttribute shouldReparse)
> {
>     if (shouldReparse) {
>@@ -197,10 +173,10 @@ void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, Sh
> 
> void StyledElement::parseAttribute(const Attribute& attribute)
> {
>-    if (attribute.name() == classAttr)
>-        classAttributeChanged(attribute.value());
>-    else if (attribute.name() == styleAttr)
>+    if (attribute.name() == styleAttr)
>         styleAttributeChanged(attribute.value());
>+    else
>+        Element::parseAttribute(attribute);
> }
> 
> void StyledElement::inlineStyleChanged()
>diff --git a/Source/WebCore/dom/StyledElement.h b/Source/WebCore/dom/StyledElement.h
>index a080dc1361b2d8bb1922ca6bae3aed0a9d49563b..66418ddfdc5912add988c8f59ec15072b83e8c6a 100644
>--- a/Source/WebCore/dom/StyledElement.h
>+++ b/Source/WebCore/dom/StyledElement.h
>@@ -53,8 +53,6 @@ public:
> 
>     const StylePropertySet* attributeStyle();
> 
>-    const SpaceSplitString& classNames() const;
>-
>     virtual void collectStyleForAttribute(const Attribute&, StylePropertySet*) { }
> 
>     // May be called by ElementAttributeData::cloneDataFrom().
>@@ -65,7 +63,7 @@ protected:
>     StyledElement(const QualifiedName&, Document*, ConstructionType);
> 
>     virtual void attributeChanged(const Attribute&) OVERRIDE;
>-    virtual void parseAttribute(const Attribute&);
>+    virtual void parseAttribute(const Attribute&) OVERRIDE;
> 
>     virtual bool isPresentationAttribute(const QualifiedName&) const { return false; }
> 
>@@ -75,11 +73,6 @@ protected:
> 
>     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
> 
>-    // classAttributeChanged() exists to share code between
>-    // parseAttribute (called via setAttribute()) and
>-    // svgAttributeChanged (called when element.className.baseValue is set)
>-    void classAttributeChanged(const AtomicString& newClassString);
>-
> private:
>     virtual void updateStyleAttribute() const;
>     void inlineStyleChanged();
>@@ -94,13 +87,6 @@ private:
>     }
> };
> 
>-inline const SpaceSplitString& StyledElement::classNames() const
>-{
>-    ASSERT(hasClass());
>-    ASSERT(attributeData());
>-    return attributeData()->classNames();
>-}
>-
> inline void StyledElement::invalidateStyleAttribute()
> {
>     clearIsStyleAttributeValid();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index dc2f035f855fb4e389fd1072c11555426b3c239c..e93af764d3f24acab5801c08d9355ecebb8ddcec 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2012-08-22  Erik Arvidsson  <arv@chromium.org>
>+
>+        Class attribute logic needs to be moved to Element (from StyledElement)
>+        https://bugs.webkit.org/show_bug.cgi?id=93665
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Before this change classAttributeChanged was only called for StyledElement. With this refactoring
>+        it gets called for all Elements when the class attribute changes.
>+
>+        * fast/dom/Element/class-list-update-expected.txt: Added.
>+        * fast/dom/Element/class-list-update.html: Added.
>+
> 2012-08-15  Levi Weintraub  <leviw@chromium.org>
> 
>         Accumulate sub-pixel offsets through layers and transforms
>diff --git a/LayoutTests/fast/dom/Element/class-list-update-expected.txt b/LayoutTests/fast/dom/Element/class-list-update-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..27656a97c3558e9c97bb0559fb601d74439f80da
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Element/class-list-update-expected.txt
>@@ -0,0 +1,15 @@
>+Tests that modifying the class attribute updates the classList on non HTMLElements
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS htmlElement.classList.length is 1
>+PASS htmlElement.classList.length is 0
>+PASS svgElement.classList.length is 1
>+PASS svgElement.classList.length is 0
>+PASS xmlElement.classList.length is 1
>+PASS xmlElement.classList.length is 0
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/dom/Element/class-list-update.html b/LayoutTests/fast/dom/Element/class-list-update.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..0736eb44086fc80695dd4911eea4b7a7d21f64c6
>--- /dev/null
>+++ b/LayoutTests/fast/dom/Element/class-list-update.html
>@@ -0,0 +1,24 @@
>+<!DOCTYPE html>
>+<script src="../../js/resources/js-test-pre.js"></script>
>+<script>
>+description('Tests that modifying the class attribute updates the classList on non HTMLElements');
>+
>+function updateAndTestClassAttr(varName) {
>+    eval(varName + '.setAttribute("class", "' + varName + '")');
>+    shouldBe(varName + '.classList.length', '1');
>+    eval(varName + '.setAttribute("class", "")');
>+    shouldBe(varName + '.classList.length', '0');
>+}
>+
>+var htmlElement = document.createElement('div');
>+updateAndTestClassAttr('htmlElement');
>+
>+var svgElement = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
>+updateAndTestClassAttr('svgElement');
>+
>+var xmlElement = document.createElementNS('http://www.example.com', 'xml');
>+updateAndTestClassAttr('xmlElement');
>+
>+
>+</script>
>+<script src="../../js/resources/js-test-post.js"></script>

Actions: View | Formatted Diff | Diff

Attachments on bug 93665: 158898 | 158920 | 159948 | 159956 | 159965 | 159983 | 159990 | 159991