Changeset 94887 in webkit

Timestamp:

Sep 9, 2011 4:22:49 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

CSS rules not being applied when a hidden field is inserted between an input[type=checkbox] and a label
​https://bugs.webkit.org/show_bug.cgi?id=66887

Patch by Kulanthaivel Palanichamy <​kulanthaivel@codeaurora.org> on 2011-09-09
Reviewed by David Hyatt.

Source/WebCore:

Test: fast/css/adjacent-sibling-selector.html

This patch addresses the problem of elements not getting their style recomputed
when they are affected by direct adjacent sibling rules and one of their sibling in
their corresponding rules is modified dynamically.

• css/CSSStyleSelector.cpp:

(WebCore::CSSStyleSelector::canShareStyleWithElement):
(WebCore::parentStylePreventsSharing):

• css/SelectorChecker.cpp:

(WebCore::SelectorChecker::checkSelector):

• dom/Element.cpp:

(WebCore::Element::recalcStyle):
(WebCore::checkForSiblingStyleChanges):

• rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::RenderStyle):

• rendering/style/RenderStyle.h:

(WebCore::InheritedFlags::affectedByDirectAdjacentRules):
(WebCore::InheritedFlags::setAffectedByDirectAdjacentRules):

LayoutTests:

• fast/css/adjacent-sibling-selector-expected.txt: Added.
• fast/css/adjacent-sibling-selector.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/adjacent-sibling-selector-expected.txt (added)
• LayoutTests/fast/css/adjacent-sibling-selector.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSStyleSelector.cpp (modified) (2 diffs)
• Source/WebCore/css/SelectorChecker.cpp (modified) (1 diff)
• Source/WebCore/dom/Element.cpp (modified) (6 diffs)
• Source/WebCore/rendering/style/RenderStyle.cpp (modified) (3 diffs)
• Source/WebCore/rendering/style/RenderStyle.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-09-09  Kulanthaivel Palanichamy  <kulanthaivel@codeaurora.org>
+
+        CSS rules not being applied when a hidden field is inserted between an input[type=checkbox] and a label
+        https://bugs.webkit.org/show_bug.cgi?id=66887
+
+        Reviewed by David Hyatt.
+
+        * fast/css/adjacent-sibling-selector-expected.txt: Added.
+        * fast/css/adjacent-sibling-selector.html: Added.
+
 2011-09-09  Tom Sepez  <tsepez@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-09  Kulanthaivel Palanichamy  <kulanthaivel@codeaurora.org>
+
+        CSS rules not being applied when a hidden field is inserted between an input[type=checkbox] and a label
+        https://bugs.webkit.org/show_bug.cgi?id=66887
+
+        Reviewed by David Hyatt.
+
+        Test: fast/css/adjacent-sibling-selector.html
+
+        This patch addresses the problem of elements not getting their style recomputed
+        when they are affected by direct adjacent sibling rules and one of their sibling in
+        their corresponding rules is modified dynamically.
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::canShareStyleWithElement):
+        (WebCore::parentStylePreventsSharing):
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::checkSelector):
+        * dom/Element.cpp:
+        (WebCore::Element::recalcStyle):
+        (WebCore::checkForSiblingStyleChanges):
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::RenderStyle):
+        * rendering/style/RenderStyle.h:
+        (WebCore::InheritedFlags::affectedByDirectAdjacentRules):
+        (WebCore::InheritedFlags::setAffectedByDirectAdjacentRules):
+
 2011-09-09  Rafael Antognolli  <antognolli@profusion.mobi>
 

trunk/Source/WebCore/css/CSSStyleSelector.cpp

@@ -949 +949 @@
         return false;
 
+    if (style->affectedByDirectAdjacentRules())
+        return false;
+
 #if USE(ACCELERATED_COMPOSITING)
     // Turn off style sharing for elements that can gain layers for reasons outside of the style system.
@@ -988 +991 @@
     return parentStyle->childrenAffectedByPositionalRules() 
         || parentStyle->childrenAffectedByFirstChildRules()
-        || parentStyle->childrenAffectedByLastChildRules() 
-        || parentStyle->childrenAffectedByDirectAdjacentRules();
+        || parentStyle->childrenAffectedByLastChildRules();
 }
 

trunk/Source/WebCore/css/SelectorChecker.cpp

@@ -342 +342 @@
     case CSSSelector::DirectAdjacent:
         {
-            if (!m_isCollectingRulesOnly && e->parentNode() && e->parentNode()->isElementNode()) {
-                RenderStyle* parentStyle = elementStyle ? elementParentStyle : e->parentNode()->renderStyle();
-                if (parentStyle)
-                    parentStyle->setChildrenAffectedByDirectAdjacentRules();
+            if (!m_isCollectingRulesOnly) {
+                RenderStyle* currentStyle = elementStyle ? elementStyle : e->renderStyle();
+                if (currentStyle)
+                    currentStyle->setAffectedByDirectAdjacentRules();
             }
             Node* n = e->previousSibling();

trunk/Source/WebCore/dom/Element.cpp

@@ -1070 +1070 @@
     RefPtr<RenderStyle> currentStyle(renderStyle());
     bool hasParentStyle = parentNodeForRenderingAndStyle() ? static_cast<bool>(parentNodeForRenderingAndStyle()->renderStyle()) : false;
-    bool hasDirectAdjacentRules = currentStyle && currentStyle->childrenAffectedByDirectAdjacentRules();
     bool hasIndirectAdjacentRules = currentStyle && currentStyle->childrenAffectedByForwardPositionalRules();
 
@@ -1112 +1111 @@
             if (currentStyle->childrenAffectedByLastChildRules())
                 newStyle->setChildrenAffectedByLastChildRules();
-            if (currentStyle->childrenAffectedByDirectAdjacentRules())
-                newStyle->setChildrenAffectedByDirectAdjacentRules();
+            if (currentStyle->affectedByDirectAdjacentRules())
+                newStyle->setAffectedByDirectAdjacentRules();
         }
 
@@ -1143 +1142 @@
     // For now we will just worry about the common case, since it's a lot trickier to get the second case right
     // without doing way too much re-resolution.
-    bool forceCheckOfNextElementSibling = false;
+    bool previousSiblingHadDirectAdjacentRules = false;
     bool forceCheckOfAnyElementSibling = false;
     for (Node *n = firstChild(); n; n = n->nextSibling()) {
@@ -1155 +1154 @@
         Element* element = static_cast<Element*>(n);
         bool childRulesChanged = element->needsStyleRecalc() && element->styleChangeType() == FullStyleChange;
-        if ((forceCheckOfNextElementSibling || forceCheckOfAnyElementSibling))
+        bool childAffectedByDirectAdjacentRules = element->renderStyle() ? element->renderStyle()->affectedByDirectAdjacentRules() : previousSiblingHadDirectAdjacentRules;
+        if (childAffectedByDirectAdjacentRules || forceCheckOfAnyElementSibling)
             element->setNeedsStyleRecalc();
         if (change >= Inherit || element->childNeedsStyleRecalc() || element->needsStyleRecalc()) {
@@ -1161 +1161 @@
             element->recalcStyle(change);
         }
-        forceCheckOfNextElementSibling = childRulesChanged && hasDirectAdjacentRules;
+        previousSiblingHadDirectAdjacentRules = childAffectedByDirectAdjacentRules;
         forceCheckOfAnyElementSibling = forceCheckOfAnyElementSibling || (childRulesChanged && hasIndirectAdjacentRules);
     }
@@ -1348 +1348 @@
     }
 
-    // The + selector.  We need to invalidate the first element following the insertion point.  It is the only possible element
-    // that could be affected by this DOM change.
-    if (style->childrenAffectedByDirectAdjacentRules() && afterChange) {
-        Node* firstElementAfterInsertion = 0;
-        for (firstElementAfterInsertion = afterChange;
-             firstElementAfterInsertion && !firstElementAfterInsertion->isElementNode();
-             firstElementAfterInsertion = firstElementAfterInsertion->nextSibling()) {};
-        if (firstElementAfterInsertion && firstElementAfterInsertion->attached())
-            firstElementAfterInsertion->setNeedsStyleRecalc();
-    }
-
     // Forward positional selectors include the ~ selector, nth-child, nth-of-type, first-of-type and only-of-type.
     // Backward positional selectors include nth-last-child, nth-last-of-type, last-of-type and only-of-type.

trunk/Source/WebCore/rendering/style/RenderStyle.cpp

@@ -78 +78 @@
     , m_childrenAffectedByFirstChildRules(false)
     , m_childrenAffectedByLastChildRules(false)
-    , m_childrenAffectedByDirectAdjacentRules(false)
     , m_childrenAffectedByForwardPositionalRules(false)
     , m_childrenAffectedByBackwardPositionalRules(false)
     , m_firstChildState(false)
     , m_lastChildState(false)
+    , m_affectedByDirectAdjacentRules(false)
     , m_childIndex(0)
     , m_box(defaultStyle()->m_box)
@@ -105 +105 @@
     , m_childrenAffectedByFirstChildRules(false)
     , m_childrenAffectedByLastChildRules(false)
-    , m_childrenAffectedByDirectAdjacentRules(false)
     , m_childrenAffectedByForwardPositionalRules(false)
     , m_childrenAffectedByBackwardPositionalRules(false)
     , m_firstChildState(false)
     , m_lastChildState(false)
+    , m_affectedByDirectAdjacentRules(false)
     , m_childIndex(0)
 {
@@ -142 +142 @@
     , m_childrenAffectedByFirstChildRules(false)
     , m_childrenAffectedByLastChildRules(false)
-    , m_childrenAffectedByDirectAdjacentRules(false)
     , m_childrenAffectedByForwardPositionalRules(false)
     , m_childrenAffectedByBackwardPositionalRules(false)
     , m_firstChildState(false)
     , m_lastChildState(false)
+    , m_affectedByDirectAdjacentRules(false)
     , m_childIndex(0)
     , m_box(o.m_box)

trunk/Source/WebCore/rendering/style/RenderStyle.h

@@ -133 +133 @@
     bool m_childrenAffectedByFirstChildRules : 1;
     bool m_childrenAffectedByLastChildRules : 1;
-    bool m_childrenAffectedByDirectAdjacentRules : 1;
     bool m_childrenAffectedByForwardPositionalRules : 1;
     bool m_childrenAffectedByBackwardPositionalRules : 1;
     bool m_firstChildState : 1;
     bool m_lastChildState : 1;
+    bool m_affectedByDirectAdjacentRules : 1;
     unsigned m_childIndex : 21; // Plenty of bits to cache an index.
 
@@ -1263 +1263 @@
     void setAffectedByAttributeSelectors() { m_affectedByAttributeSelectors = true; }
 
+    bool affectedByDirectAdjacentRules() const { return m_affectedByDirectAdjacentRules; }
+    void setAffectedByDirectAdjacentRules() { m_affectedByDirectAdjacentRules = true; }
+
     bool unique() const { return m_unique; }
     void setUnique() { m_unique = true; }
@@ -1275 +1278 @@
     bool childrenAffectedByLastChildRules() const { return m_childrenAffectedByLastChildRules; }
     void setChildrenAffectedByLastChildRules() { m_childrenAffectedByLastChildRules = true; }
-    bool childrenAffectedByDirectAdjacentRules() const { return m_childrenAffectedByDirectAdjacentRules; }
-    void setChildrenAffectedByDirectAdjacentRules() { m_childrenAffectedByDirectAdjacentRules = true; }
     bool childrenAffectedByForwardPositionalRules() const { return m_childrenAffectedByForwardPositionalRules; }
     void setChildrenAffectedByForwardPositionalRules() { m_childrenAffectedByForwardPositionalRules = true; }