2010-05-01 Maciej Stachowiak <mjs@apple.com>
authormjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 1 May 2010 20:23:35 +0000 (20:23 +0000)
committermjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 1 May 2010 20:23:35 +0000 (20:23 +0000)
        Reviewed by Sam Weinig.

        REGRESSION (r58273): Visited links do not change color immediately when Cmd-clicked
        https://bugs.webkit.org/show_bug.cgi?id=38422
        <rdar://problem/7921778>

        Tests:
            manual-tests/visited-link-new-window.html

        * css/CSSStyleSelector.cpp:
        (WebCore::CSSStyleSelector::initElement): Only cache the visited link state
        when invoked as part of a helper call to styleForElement or pseudoStyleForElement,
        to avoid caching the visited link state beyond the scope of a single style lookup.
        (WebCore::CSSStyleSelector::styleForElement): Adjust for above change.
        (WebCore::CSSStyleSelector::pseudoStyleForElement): Adjust for above change.
        * css/CSSStyleSelector.h:
        (WebCore::CSSStyleSelector::styleForElement): Change so "visited link helper mode"
        can't accidentally be called from outside CSSStyleSelector itself.
        (WebCore::CSSStyleSelector::pseudoStyleForElement): ditto
        * manual-tests/visited-link-new-window.html: Added. I could not figure out any way
        to make an automated test that supports visited link coloring.

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

WebCore/ChangeLog
WebCore/css/CSSStyleSelector.cpp
WebCore/css/CSSStyleSelector.h
WebCore/manual-tests/visited-link-new-window.html [new file with mode: 0644]

index 54ae5b3c92d510f73b7a7171279951bb6abdb651..ad13715675d45c761c49015f493b8122ecafb127 100644 (file)
@@ -1,3 +1,27 @@
+2010-05-01  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        REGRESSION (r58273): Visited links do not change color immediately when Cmd-clicked
+        https://bugs.webkit.org/show_bug.cgi?id=38422
+        <rdar://problem/7921778>
+
+        Tests:
+            manual-tests/visited-link-new-window.html
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::initElement): Only cache the visited link state
+        when invoked as part of a helper call to styleForElement or pseudoStyleForElement,
+        to avoid caching the visited link state beyond the scope of a single style lookup.
+        (WebCore::CSSStyleSelector::styleForElement): Adjust for above change.
+        (WebCore::CSSStyleSelector::pseudoStyleForElement): Adjust for above change.
+        * css/CSSStyleSelector.h:
+        (WebCore::CSSStyleSelector::styleForElement): Change so "visited link helper mode"
+        can't accidentally be called from outside CSSStyleSelector itself.
+        (WebCore::CSSStyleSelector::pseudoStyleForElement): ditto
+        * manual-tests/visited-link-new-window.html: Added. I could not figure out any way
+        to make an automated test that supports visited link coloring.
+
 2010-05-01  Yael Aharon  <yael.aharon@nokia.com>
 
         Reviewed by Darin Adler.
index 927ef917f3b0448b43b7544d86ced60ead50639e..916083b47d72f726d98d069cf3cce6138909ea18 100644 (file)
@@ -806,9 +806,9 @@ void CSSStyleSelector::sortMatchedRules(unsigned start, unsigned end)
         m_matchedRules[i] = rulesMergeBuffer[i - start];
 }
 
-inline void CSSStyleSelector::initElement(Element* e)
+inline void CSSStyleSelector::initElement(Element* e, bool helperCallForVisitedStyle = false)
 {
-    if (m_element != e)
+    if (!helperCallForVisitedStyle)
         m_haveCachedLinkState = false;
 
     m_element = e;
@@ -1134,7 +1134,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForDocument(Document* document)
 // If resolveForRootDefault is true, style based on user agent style sheet only. This is used in media queries, where
 // relative units are interpreted according to document root element style, styled only with UA stylesheet
 
-PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyle* defaultParent, bool allowSharing, bool resolveForRootDefault, bool matchVisitedRules)
+PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyle* defaultParent, bool allowSharing, bool resolveForRootDefault, bool helperCallForVisitedStyle)
 {
     // Once an element has a renderer, we don't try to destroy it, since otherwise the renderer
     // will vanish if a style recalc happens during loading.
@@ -1150,7 +1150,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
         return s_styleNotYetAvailable;
     }
 
-    initElement(e);
+    initElement(e, helperCallForVisitedStyle);
     if (allowSharing) {
         RenderStyle* sharedStyle = locateSharedStyle();
         if (sharedStyle)
@@ -1160,7 +1160,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
 
     // Compute our style allowing :visited to match first.
     RefPtr<RenderStyle> visitedStyle;
-    if (!matchVisitedRules && m_parentStyle && (m_parentStyle->insideLink() || e->isLink()) && e->document()->usesLinkRules()) {
+    if (!helperCallForVisitedStyle && m_parentStyle && (m_parentStyle->insideLink() || e->isLink()) && e->document()->usesLinkRules()) {
         // Fetch our parent style.
         RenderStyle* parentStyle = m_parentStyle;
         if (!e->isLink()) {
@@ -1174,7 +1174,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
         initForStyleResolve(e, defaultParent);
     }
 
-    m_checker.m_matchVisitedPseudoClass = matchVisitedRules;
+    m_checker.m_matchVisitedPseudoClass = helperCallForVisitedStyle;
 
     m_style = RenderStyle::create();
 
@@ -1300,7 +1300,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::styleForElement(Element* e, RenderStyl
     }
 
     // Reset the value back before applying properties, so that -webkit-link knows what color to use.
-    m_checker.m_matchVisitedPseudoClass = matchVisitedRules;
+    m_checker.m_matchVisitedPseudoClass = helperCallForVisitedStyle;
     
     // Now we have all of the matched rules in the appropriate order.  Walk the rules and apply
     // high-priority properties first, i.e., those properties that other properties depend on.
@@ -1447,7 +1447,7 @@ void CSSStyleSelector::keyframeStylesForAnimation(Element* e, const RenderStyle*
         list.clear();
 }
 
-PassRefPtr<RenderStyle> CSSStyleSelector::pseudoStyleForElement(PseudoId pseudo, Element* e, RenderStyle* parentStyle, bool matchVisitedLinks)
+PassRefPtr<RenderStyle> CSSStyleSelector::pseudoStyleForElement(PseudoId pseudo, Element* e, RenderStyle* parentStyle, bool helperCallForVisitedStyle)
 {
     if (!e)
         return 0;
@@ -1455,7 +1455,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::pseudoStyleForElement(PseudoId pseudo,
     // Compute our :visited style first, so that we know whether or not we'll need to create a normal style just to hang it
     // off of.
     RefPtr<RenderStyle> visitedStyle;
-    if (!matchVisitedLinks && parentStyle && parentStyle->insideLink()) {
+    if (!helperCallForVisitedStyle && parentStyle && parentStyle->insideLink()) {
         // Fetch our parent style with :visited in effect.
         RenderStyle* parentVisitedStyle = parentStyle->getCachedPseudoStyle(VISITED_LINK);
         visitedStyle = pseudoStyleForElement(pseudo, e, parentVisitedStyle ? parentVisitedStyle : parentStyle, true);
@@ -1463,11 +1463,11 @@ PassRefPtr<RenderStyle> CSSStyleSelector::pseudoStyleForElement(PseudoId pseudo,
             visitedStyle->setStyleType(VISITED_LINK);
     }
 
-    initElement(e);
+    initElement(e, helperCallForVisitedStyle);
     initForStyleResolve(e, parentStyle, pseudo);
     m_style = parentStyle;
     
-    m_checker.m_matchVisitedPseudoClass = matchVisitedLinks;
+    m_checker.m_matchVisitedPseudoClass = helperCallForVisitedStyle;
 
     // Since we don't use pseudo-elements in any of our quirk/print user agent rules, don't waste time walking
     // those rules.
@@ -1493,7 +1493,7 @@ PassRefPtr<RenderStyle> CSSStyleSelector::pseudoStyleForElement(PseudoId pseudo,
     m_lineHeightValue = 0;
     
     // Reset the value back before applying properties, so that -webkit-link knows what color to use.
-    m_checker.m_matchVisitedPseudoClass = matchVisitedLinks;
+    m_checker.m_matchVisitedPseudoClass = helperCallForVisitedStyle;
 
     // High-priority properties.
     applyDeclarations(true, false, 0, m_matchedDecls.size() - 1);
index 3f4b03bd815e95eef308ef70bb9777c9a09bdb28..b17178265f3596f30f19c0ff2a6bdd9d4ab148c0 100644 (file)
@@ -87,10 +87,10 @@ public:
         ~CSSStyleSelector();
 
         void initForStyleResolve(Element*, RenderStyle* parentStyle = 0, PseudoId = NOPSEUDO);
-        PassRefPtr<RenderStyle> styleForElement(Element*, RenderStyle* parentStyle = 0, bool allowSharing = true, bool resolveForRootDefault = false, bool matchVisitedLinks = false);
+        PassRefPtr<RenderStyle> styleForElement(Element* e, RenderStyle* parentStyle = 0, bool allowSharing = true, bool resolveForRootDefault = false) { return styleForElement(e, parentStyle, allowSharing, resolveForRootDefault, false); }
         void keyframeStylesForAnimation(Element*, const RenderStyle*, KeyframeList& list);
 
-        PassRefPtr<RenderStyle> pseudoStyleForElement(PseudoId, Element*, RenderStyle* parentStyle = 0, bool matchVisitedLinks = false);
+        PassRefPtr<RenderStyle> pseudoStyleForElement(PseudoId pseudo, Element* e, RenderStyle* parentStyle = 0) { return pseudoStyleForElement(pseudo, e, parentStyle, false); }
 
         static PassRefPtr<RenderStyle> styleForDocument(Document*);
 
@@ -101,7 +101,9 @@ public:
 #endif
 
     private:
-        void initElement(Element*);
+        PassRefPtr<RenderStyle> styleForElement(Element*, RenderStyle* parentStyle, bool allowSharing, bool resolveForRootDefault, bool helperCallForVisitedStyle);
+        PassRefPtr<RenderStyle> pseudoStyleForElement(PseudoId, Element*, RenderStyle*, bool helperCallForVisitedStyle);
+        void initElement(Element*, bool);
         RenderStyle* locateSharedStyle();
         Node* locateCousinList(Element* parent, unsigned depth = 1);
         bool canShareStyleWithElement(Node*);
diff --git a/WebCore/manual-tests/visited-link-new-window.html b/WebCore/manual-tests/visited-link-new-window.html
new file mode 100644 (file)
index 0000000..5e10a34
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Visited link coloring test</test>
+<style>
+:visited {
+    color: limegreen;
+}
+</style>
+</head>
+<body>
+<h1>Visited link coloring test</h1>
+<p>Make sure the link below is not colored as visited (delete the item
+for visited-link.html from your history and restart if
+necessary). Then cmd-click it. The link should turn lime green.</p>
+<a href="resources/visited-link.html">Am I a visited link?</a>
+</body>
+</html>