2007-10-11 Allan Sandfeld Jensen <sandfeld@kde.org>
authoroliver <oliver@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Oct 2007 15:40:08 +0000 (15:40 +0000)
committeroliver <oliver@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Oct 2007 15:40:08 +0000 (15:40 +0000)
         Reviewed by Maciej and Eric.

         Implement CSS selector combinators nondeterministic matching.
         Fixes http://bugs.webkit.org/show_bug.cgi?id=3428

         * css/CSSStyleSelector.cpp:
         (WebCore::CSSStyleSelector::matchRulesForList):
         (WebCore::CSSStyleSelector::checkSelector): Split the function and make the second part recursive
         (WebCore::CSSStyleSelector::checkOneSelector): Handle pseodo-elements rules and hoveractive quirks
         here instead of in checkSelector.
         * css/CSSStyleSelector.h:

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

LayoutTests/ChangeLog
LayoutTests/fast/selectors/nondeterministic-combinators.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/css/CSSStyleSelector.cpp
WebCore/css/CSSStyleSelector.h

index f83557d6f53bae2e824e2a782537ed0fb30dbdfe..fbd5fac46e73eda23d7b5361e57d60b52f4f7a05 100644 (file)
@@ -1,3 +1,14 @@
+2007-10-11  Allan Sandfeld Jensen  <sandfeld@kde.org>
+         Reviewed by Maciej and Eric.
+         Test for nondeterministic matching of CSS selectors, see http://bugs.webkit.org/show_bug.cgi?id=3428
+         * fast/selectors/nondeterministic-combinators.html: Added.
+         * platform/mac/fast/selectors/nondeterministic-combinators-expected.checksum
+         * platform/mac/fast/selectors/nondeterministic-combinators-expected.png
+         * platform/mac/fast/selectors/nondeterministic-combinators-expected.txt
 2007-10-10  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Nikolas Zimmermann.
diff --git a/LayoutTests/fast/selectors/nondeterministic-combinators.html b/LayoutTests/fast/selectors/nondeterministic-combinators.html
new file mode 100644 (file)
index 0000000..36eced3
--- /dev/null
@@ -0,0 +1,30 @@
+<html>
+<head>
+<style>
+.a > .b .c {
+  color: green;
+}
+
+.d + .e ~ .f {
+  color: green;
+}
+
+</style>
+</head>
+<body>
+<div class="a">
+<div class="b">
+<div class="b">
+<div class="c">
+This text should be green.
+</div>
+</div>
+</div>
+</div>
+
+<div class="d"></div>
+<div class="e"></div>
+<div class="e"></div>
+<div class="f">This text should also be green.</div>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.checksum b/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.checksum
new file mode 100644 (file)
index 0000000..b70bd52
--- /dev/null
@@ -0,0 +1 @@
+63db3cb95ce5ef513112fbe7ed6fd021
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.png b/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.png
new file mode 100644 (file)
index 0000000..0003245
Binary files /dev/null and b/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.txt b/LayoutTests/platform/mac/fast/selectors/nondeterministic-combinators-expected.txt
new file mode 100644 (file)
index 0000000..c31e69b
--- /dev/null
@@ -0,0 +1,17 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {DIV} at (0,0) size 784x18
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderBlock {DIV} at (0,0) size 784x18
+            RenderBlock {DIV} at (0,0) size 784x18 [color=#008000]
+              RenderText {#text} at (0,0) size 163x18
+                text run at (0,0) width 163: "This text should be green."
+      RenderBlock {DIV} at (0,18) size 784x0
+      RenderBlock {DIV} at (0,18) size 784x0
+      RenderBlock {DIV} at (0,18) size 784x0
+      RenderBlock {DIV} at (0,18) size 784x18 [color=#008000]
+        RenderText {#text} at (0,0) size 192x18
+          text run at (0,0) width 192: "This text should also be green."
index c3202e1eb9d6f51ad1e633fda57a8ef962d7f438..c47a7d599dc1b44b2abac22d6f14728c2deb18b5 100644 (file)
@@ -1,3 +1,17 @@
+2007-10-11  Allan Sandfeld Jensen  <sandfeld@kde.org>
+         Reviewed by Maciej and Eric.
+         Implement CSS selector combinators nondeterministic matching. 
+         Fixes http://bugs.webkit.org/show_bug.cgi?id=3428
+         * css/CSSStyleSelector.cpp:
+         (WebCore::CSSStyleSelector::matchRulesForList):
+         (WebCore::CSSStyleSelector::checkSelector): Split the function and make the second part recursive
+         (WebCore::CSSStyleSelector::checkOneSelector): Handle pseodo-elements rules and hoveractive quirks 
+         here instead of in checkSelector.
+         * css/CSSStyleSelector.h:
 2007-10-10  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Nikolas Zimmermann.
index 1cc446c10e2b0ffc88c8941e755038d183cc8436..c7f2dae32e27e8b60fcf88924dc1eb26ae259e3e 100644 (file)
@@ -408,7 +408,7 @@ void CSSStyleSelector::matchRulesForList(CSSRuleDataList* rules, int& firstRuleI
         CSSStyleRule* rule = d->rule();
         const AtomicString& localName = element->localName();
         const AtomicString& selectorLocalName = d->selector()->m_tag.localName();
-        if ((localName == selectorLocalName || selectorLocalName == starAtom) && checkSelector(d->selector(), element)) {
+        if ((localName == selectorLocalName || selectorLocalName == starAtom) && checkSelector(d->selector())) {
             // If the rule has no properties to apply, then ignore it.
             CSSMutableStyleDeclaration* decl = rule->declaration();
             if (!decl || !decl->length())
@@ -1199,123 +1199,99 @@ RefPtr<CSSRuleList> CSSStyleSelector::pseudoStyleRulesForElement(Element* e, Str
     return 0;
 }
 
-static bool subject;
-
-bool CSSStyleSelector::checkSelector(CSSSelector* sel, Element *e)
+bool CSSStyleSelector::checkSelector(CSSSelector* sel)
 {
     dynamicPseudo = RenderStyle::NOPSEUDO;
-    
-    Node *n = e;
-
-    // we have the subject part of the selector
-    subject = true;
-
-    // We track whether or not the rule contains only :hover and :active in a simple selector. If
-    // so, we can't allow that to apply to every element on the page.  We assume the author intended
-    // to apply the rules only to links.
-    bool onlyHoverActive = (!sel->hasTag() &&
-                            (sel->m_match == CSSSelector::PseudoClass &&
-                              (sel->pseudoType() == CSSSelector::PseudoHover ||
-                               sel->pseudoType() == CSSSelector::PseudoActive)));
-    bool affectedByHover = style ? style->affectedByHoverRules() : false;
-    bool affectedByActive = style ? style->affectedByActiveRules() : false;
-    bool havePseudo = pseudoStyle != RenderStyle::NOPSEUDO;
-    
-    // first selector has to match
-    if (!checkOneSelector(sel, e))
+
+    // Check the selector
+    SelectorMatch match = checkSelector(sel, element, true, false);
+    if(match != SelectorMatches) return false;
+
+    if (pseudoStyle != RenderStyle::NOPSEUDO && pseudoStyle != dynamicPseudo)
         return false;
 
-    // check the subselectors
+    return true;
+}
+
+// Recursive check of selectors and combinators
+// It can return 3 different values:
+// * SelectorMatches         - the selector matches the element e
+// * SelectorFailsLocally    - the selector fails for the element e
+// * SelectorFailsCompletely - the selector fails for e and any sibling or ancestor of e
+CSSStyleSelector::SelectorMatch CSSStyleSelector::checkSelector(CSSSelector* sel, Element *e, bool isAncestor, bool isSubSelector)
+{
+    // first selector has to match
+    if (!checkOneSelector(sel, e, isAncestor, isSubSelector))
+        return SelectorFailsLocally;
+
+    // The rest of the selectors has to match
     CSSSelector::Relation relation = sel->relation();
-    while((sel = sel->m_tagHistory)) {
-        if (!n->isElementNode())
-            return false;
-        if (relation != CSSSelector::SubSelector) {
-            subject = false;
-            if (havePseudo && dynamicPseudo != pseudoStyle)
-                return false;
-        }
-        
-        switch(relation)
-        {
+
+    // Prepare next sel
+    sel = sel->m_tagHistory;
+    if (!sel) return SelectorMatches;
+
+    if (relation != CSSSelector::SubSelector)
+        // Bail-out if this selector is irrelevant for the pseudoStyle
+        if (pseudoStyle != RenderStyle::NOPSEUDO && pseudoStyle != dynamicPseudo)
+            return SelectorFailsCompletely;
+
+    switch(relation) {
         case CSSSelector::Descendant:
-            // FIXME: This match needs to know how to backtrack and be non-deterministic.
-            do {
-                n = n->parentNode();
+            while(true)
+            {
+                Node* n = e->parentNode();
                 if (!n || !n->isElementNode())
-                    return false;
-            } while (!checkOneSelector(sel, static_cast<Element*>(n)));
+                    return SelectorFailsCompletely;
+                e = static_cast<Element*>(n);
+                SelectorMatch match = checkSelector(sel, e, true, false);
+                if (match != SelectorFailsLocally)
+                    return match;
+            }
             break;
         case CSSSelector::Child:
         {
-            n = n->parentNode();
+            Node* n = e->parentNode();
             if (!n || !n->isElementNode())
-                return false;
-            if (!checkOneSelector(sel, static_cast<Element*>(n)))
-                return false;
-            break;
+                return SelectorFailsCompletely;
+            e = static_cast<Element*>(n);
+            return checkSelector(sel, e, true, false);
         }
         case CSSSelector::DirectAdjacent:
         {
-            n = n->previousSibling();
+            Node* n = e->previousSibling();
             while (n && !n->isElementNode())
                 n = n->previousSibling();
             if (!n)
-                return false;
-            if (!checkOneSelector(sel, static_cast<Element*>(n)))
-                return false;
-            break;
+                return SelectorFailsLocally;
+            e = static_cast<Element*>(n);
+            return checkSelector(sel, e, false, false); 
         }
         case CSSSelector::IndirectAdjacent:
-            // FIXME: This match needs to know how to backtrack and be non-deterministic.
-            do {
-                n = n->previousSibling();
-                while (n && !n->isElementNode())
+            while(true)
+            {
+                Node* n = e->previousSibling();
+                while(n && !n->isElementNode())
                     n = n->previousSibling();
                 if (!n)
-                    return false;
-            } while (!checkOneSelector(sel, static_cast<Element*>(n)));
-            break;
-       case CSSSelector::SubSelector:
-       {
-            if (onlyHoverActive)
-                onlyHoverActive = (sel->m_match == CSSSelector::PseudoClass &&
-                                   (sel->pseudoType() == CSSSelector::PseudoHover ||
-                                    sel->pseudoType() == CSSSelector::PseudoActive));
-            
-            Element *elem = static_cast<Element*>(n);
-            // a selector is invalid if something follows :first-xxx
-            if (elem == element && dynamicPseudo != RenderStyle::NOPSEUDO)
-                return false;
-            if (!checkOneSelector(sel, elem, true))
-                return false;
+                    return SelectorFailsLocally;
+                e = static_cast<Element*>(n);
+                SelectorMatch match = checkSelector(sel, e, false, false);
+                if (match != SelectorFailsLocally)
+                    return match;
+            };
             break;
-        }
-        }
-        relation = sel->relation();
-    }
-
-    if (subject && havePseudo && dynamicPseudo != pseudoStyle)
-        return false;
-    
-    // disallow *:hover, *:active, and *:hover:active except for links
-    if (!strictParsing && onlyHoverActive && subject) {
-        if (pseudoState == PseudoUnknown)
-            checkPseudoState(e);
-
-        if (pseudoState == PseudoNone) {
-            if (!affectedByHover && style->affectedByHoverRules())
-                style->setAffectedByHoverRules(false);
-            if (!affectedByActive && style->affectedByActiveRules())
-                style->setAffectedByActiveRules(false);
-            return false;
-        }
+        case CSSSelector::SubSelector:
+            // a selector is invalid if something follows a pseudo-element
+            if (e == element && dynamicPseudo != RenderStyle::NOPSEUDO)
+                return SelectorFailsCompletely;
+            return checkSelector(sel, e, isAncestor, true);
     }
 
-    return true;
+    return SelectorFailsCompletely;
 }
 
-bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isSubSelector)
+bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isAncestor, bool isSubSelector)
 {
     if(!e)
         return false;
@@ -1405,12 +1381,10 @@ bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isSub
             break;
         }
     }
-    if(sel->m_match == CSSSelector::PseudoClass || sel->m_match == CSSSelector::PseudoElement)
+    if(sel->m_match == CSSSelector::PseudoClass
     {
-        // Pseudo elements. We need to check first child here. No dynamic pseudo
-        // elements for the moment
-            switch (sel->pseudoType()) {
-                // Pseudo classes:
+        switch (sel->pseudoType()) {
+            // Pseudo classes:
             case CSSSelector::PseudoEmpty:
                 if (!e->firstChild())
                     return true;
@@ -1467,19 +1441,6 @@ bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isSub
                 if (pseudoState == PseudoVisited)
                     return true;
                 break;
-            case CSSSelector::PseudoHover: {
-                // If we're in quirks mode, then hover should never match anchors with no
-                // href and *:hover should not match anything.  This is important for sites like wsj.com.
-                if (strictParsing || isSubSelector || sel->relation() == CSSSelector::SubSelector || (sel->hasTag() && !e->hasTagName(aTag)) || e->isLink()) {
-                    if (element == e && style)
-                        style->setAffectedByHoverRules(true);
-                    if (element != e && e->renderStyle())
-                        e->renderStyle()->setAffectedByHoverRules(true);
-                    if (e->hovered())
-                        return true;
-                }
-                break;
-            }
             case CSSSelector::PseudoDrag: {
                 if (element == e && style)
                     style->setAffectedByDragRules(true);
@@ -1493,10 +1454,23 @@ bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isSub
                 if (e && e->focused() && e->document()->frame()->isActive())
                     return true;
                 break;
+            case CSSSelector::PseudoHover: {
+                // If we're in quirks mode, then hover should never match anchors with no
+                // href and *:hover should not match anything.  This is important for sites like wsj.com.
+                if (strictParsing || isSubSelector || (sel->hasTag() && !e->hasTagName(aTag)) || e->isLink()) {
+                    if (element == e && style)
+                        style->setAffectedByHoverRules(true);
+                    if (element != e && e->renderStyle())
+                        e->renderStyle()->setAffectedByHoverRules(true);
+                    if (e->hovered())
+                        return true;
+                }
+                break;
+            }
             case CSSSelector::PseudoActive:
                 // If we're in quirks mode, then :active should never match anchors with no
                 // href and *:active should not match anything. 
-                if (strictParsing || isSubSelector || sel->relation() == CSSSelector::SubSelector || (sel->hasTag() && !e->hasTagName(aTag)) || e->isLink()) {
+                if (strictParsing || isSubSelector || (sel->hasTag() && !e->hasTagName(aTag)) || e->isLink()) {
                     if (element == e && style)
                         style->setAffectedByActiveRules(true);
                     else if (e->renderStyle())
@@ -1549,30 +1523,32 @@ bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isSub
                     // restriction in CSS3, but it is, so let's honour it.
                     if (subSel->m_simpleSelector)
                         break;
-                    if (!checkOneSelector(subSel, e))
+                    if (!checkOneSelector(subSel, e, isAncestor, true))
                         return true;
                 }
                 break;
             }
             case CSSSelector::PseudoUnknown:
+            case CSSSelector::PseudoNotParsed:
+            default:
                 ASSERT_NOT_REACHED();
                 break;
-            
+        }
+        return false;
+    }
+    if (sel->m_match == CSSSelector::PseudoElement) {
+        if (e != element) return false;
+
+        switch (sel->pseudoType()) {
             // Pseudo-elements:
             case CSSSelector::PseudoFirstLine:
-                if (subject) {
-                    dynamicPseudo = RenderStyle::FIRST_LINE;
-                    return true;
-                }
-                break;
+                dynamicPseudo = RenderStyle::FIRST_LINE;
+                return true;
             case CSSSelector::PseudoFirstLetter:
-                if (subject) {
-                    dynamicPseudo = RenderStyle::FIRST_LETTER;
-                    if (Document* doc = e->document())
-                        doc->setUsesFirstLetterRules(true);
-                    return true;
-                }
-                break;
+                dynamicPseudo = RenderStyle::FIRST_LETTER;
+                if (Document* doc = e->document())
+                    doc->setUsesFirstLetterRules(true);
+                return true;
             case CSSSelector::PseudoSelection:
                 dynamicPseudo = RenderStyle::SELECTION;
                 return true;
@@ -1600,8 +1576,10 @@ bool CSSStyleSelector::checkOneSelector(CSSSelector* sel, Element* e, bool isSub
             case CSSSelector::PseudoSearchResultsButton:
                 dynamicPseudo = RenderStyle::SEARCH_RESULTS_BUTTON;
                 return true;
+            case CSSSelector::PseudoUnknown:
             case CSSSelector::PseudoNotParsed:
-                ASSERT(false);
+            default:
+                ASSERT_NOT_REACHED();
                 break;
         }
         return false;
index dcadf255e6b1332c2a02d92ce6ca59b7acaa9858..cfd94c181f6fd255ec78928a96892ff912cb37a6 100644 (file)
@@ -134,10 +134,12 @@ class StyledElement;
 
         /* checks if a compound selector (which can consist of multiple simple selectors)
            matches the given Element */
-        bool checkSelector(CSSSelector* selector, Element *e);
-        
+        bool checkSelector(CSSSelector* selector);
+        enum SelectorMatch { SelectorMatches=0, SelectorFailsLocally, SelectorFailsCompletely};
+        SelectorMatch checkSelector(CSSSelector* selector, Element *e, bool isAncestor, bool isSubSelector);
+
         /* checks if the selector matches the given Element */
-        bool checkOneSelector(CSSSelector*, Element*, bool isSubSelector = false);
+        bool checkOneSelector(CSSSelector*, Element*, bool isAncestor, bool isSubSelector = false);
 
         /* This function fixes up the default font size if it detects that the
            current generic font family has changed. -dwh */