Changeset 45569 in webkit

Timestamp:

Jul 6, 2009 3:11:40 PM (15 years ago)

Author:

alice.liu@apple.com

Message:

2009-07-06 Alice Liu <​alice.liu@apple.com>

Reviewed by Darin Adler.

REGRESSION(r45285): focus rings are black on windows safari
​https://bugs.webkit.org/show_bug.cgi?id=26821
<rdar://problem/7018252>

Add a mechanism for setting a custom focus ring color, and relocate the
determination of focus ring color to base class RenderTheme.

No new tests. Already-existing tests run in pixel mode would have caught this regression

• css/CSSStyleSelector.cpp: (WebCore::CSSStyleSelector::getColorFromPrimitiveValue): Move the responsibility of determining focus color to base class RenderTheme.
• rendering/RenderTheme.cpp: (WebCore::customFocusRingColor): Added static local. (WebCore::RenderTheme::setCustomFocusRingColor): Added (WebCore::RenderTheme::focusRingColor): Use custom color over platform color.
• rendering/RenderTheme.h: (WebCore::RenderTheme::platformFocusRingColor): default implementation, black color
• rendering/RenderThemeSafari.cpp: (WebCore::RenderTheme::themeForPage): Circumstances that lead to returning the RenderThemeWin are the same under which we want to use the focus ring color from SafariTheme. (WebCore::RenderThemeSafari::platformFocusRingColor): Renamed from focusRingColor
• rendering/RenderThemeSafari.h:

Renaming focusRingColor to platformFocusRingColor in these files:

• rendering/RenderThemeChromiumMac.h:
• rendering/RenderThemeChromiumMac.mm: (WebCore::RenderThemeChromiumMac::platformFocusRingColor):
• rendering/RenderThemeChromiumSkia.cpp: (WebCore::RenderThemeChromiumSkia::platformFocusRingColor):
• rendering/RenderThemeChromiumSkia.h:
• rendering/RenderThemeMac.h:
• rendering/RenderThemeMac.mm: (WebCore::RenderThemeMac::platformFocusRingColor):

Cleanup leftover from removal of WebCore::Color in r45285 in these files:

• WebCore.order:
• platform/graphics/Color.h:

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• WebCore.order (modified) (1 diff)
• css/CSSStyleSelector.cpp (modified) (1 diff)
• platform/graphics/Color.h (modified) (1 diff)
• rendering/RenderTheme.cpp (modified) (2 diffs)
• rendering/RenderTheme.h (modified) (1 diff)
• rendering/RenderThemeChromiumMac.h (modified) (1 diff)
• rendering/RenderThemeChromiumMac.mm (modified) (1 diff)
• rendering/RenderThemeChromiumSkia.cpp (modified) (1 diff)
• rendering/RenderThemeChromiumSkia.h (modified) (1 diff)
• rendering/RenderThemeMac.h (modified) (1 diff)
• rendering/RenderThemeMac.mm (modified) (1 diff)
• rendering/RenderThemeSafari.cpp (modified) (2 diffs)
• rendering/RenderThemeSafari.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-07-06  Alice Liu  <alice.liu@apple.com>
+
+        Reviewed by Darin Adler.
+
+        REGRESSION(r45285): focus rings are black on windows safari
+        https://bugs.webkit.org/show_bug.cgi?id=26821
+        <rdar://problem/7018252>
+
+        Add a mechanism for setting a custom focus ring color, and relocate the 
+        determination of focus ring color to base class RenderTheme.
+
+        No new tests.  Already-existing tests run in pixel mode would have caught this regression
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::getColorFromPrimitiveValue): Move the responsibility
+        of determining focus color to base class RenderTheme.
+        * rendering/RenderTheme.cpp:
+        (WebCore::customFocusRingColor): Added static local.
+        (WebCore::RenderTheme::setCustomFocusRingColor): Added
+        (WebCore::RenderTheme::focusRingColor): Use custom color over platform color.
+        * rendering/RenderTheme.h:
+        (WebCore::RenderTheme::platformFocusRingColor): default implementation, black color
+        * rendering/RenderThemeSafari.cpp:
+        (WebCore::RenderTheme::themeForPage): Circumstances that lead to returning the RenderThemeWin
+        are the same under which we want to use the focus ring color from SafariTheme.
+        (WebCore::RenderThemeSafari::platformFocusRingColor): Renamed from focusRingColor
+        * rendering/RenderThemeSafari.h:
+
+        Renaming focusRingColor to platformFocusRingColor in these files:
+        * rendering/RenderThemeChromiumMac.h:
+        * rendering/RenderThemeChromiumMac.mm:
+        (WebCore::RenderThemeChromiumMac::platformFocusRingColor):
+        * rendering/RenderThemeChromiumSkia.cpp:
+        (WebCore::RenderThemeChromiumSkia::platformFocusRingColor):
+        * rendering/RenderThemeChromiumSkia.h:
+        * rendering/RenderThemeMac.h:
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::platformFocusRingColor):
+
+        Cleanup leftover from removal of WebCore::Color in r45285 in these files:
+        * WebCore.order:
+        * platform/graphics/Color.h:
+
 2009-07-06  Shinichiro Hamaji  <hamaji@chromium.org>
 

trunk/WebCore/WebCore.order

@@ -3794 +3794 @@
 __ZN7WebCore13ContainerNode8setFocusEb
 __ZN7WebCore4Node8setFocusEb
-__ZN7WebCore14focusRingColorEv
 +[WebCoreControlTintObserver controlTintDidChange]
 __ZN7WebCore26usesTestModeFocusRingColorEv

trunk/WebCore/css/CSSStyleSelector.cpp

@@ -5744 +5744 @@
             col = m_element->document()->activeLinkColor();
         else if (ident == CSSValueWebkitFocusRingColor)
-            col = RenderTheme::defaultTheme()->focusRingColor();
+            col = RenderTheme::focusRingColor();
         else if (ident == CSSValueCurrentcolor)
             col = m_style->color();

trunk/WebCore/platform/graphics/Color.h

@@ -146 +146 @@
 }
 
-Color focusRingColor();
 Color colorFromPremultipliedARGB(unsigned);
 unsigned premultipliedARGBFromColor(const Color&);

trunk/WebCore/rendering/RenderTheme.cpp

@@ -42 +42 @@
 
 using namespace HTMLNames;
+
+static Color& customFocusRingColor()
+{
+    DEFINE_STATIC_LOCAL(Color, color, ());
+    return color;
+}
 
 RenderTheme::RenderTheme()
@@ -854 +860 @@
 }
 
-Color RenderTheme::focusRingColor() const
-{
-    return Color(0, 0, 0); // Black.
+void RenderTheme::setCustomFocusRingColor(const Color& c)
+{
+    customFocusRingColor() = c;
+}
+
+Color RenderTheme::focusRingColor()
+{
+    return customFocusRingColor().isValid() ? customFocusRingColor() : defaultTheme()->platformFocusRingColor();
 }
 

trunk/WebCore/rendering/RenderTheme.h

@@ -138 +138 @@
     virtual Color platformInactiveTextSearchHighlightColor() const;
 
-    virtual Color focusRingColor() const;
+    static Color focusRingColor();
+    virtual Color platformFocusRingColor() const { return Color(0, 0, 0); }
+    static void setCustomFocusRingColor(const Color&);
 
     virtual void platformColorsDidChange();

trunk/WebCore/rendering/RenderThemeChromiumMac.h

@@ -65 +65 @@
         virtual Color activeListBoxSelectionBackgroundColor() const;
 
-        virtual Color focusRingColor() const;
+        virtual Color platformFocusRingColor() const;
         
         virtual void platformColorsDidChange();

trunk/WebCore/rendering/RenderThemeChromiumMac.mm

@@ -173 +173 @@
 }
 
-Color RenderThemeChromiumMac::focusRingColor() const
+Color RenderThemeChromiumMac::platformFocusRingColor() const
 {
     if (ChromiumBridge::layoutTestMode())

trunk/WebCore/rendering/RenderThemeChromiumSkia.cpp

@@ -163 +163 @@
 }
 
-Color RenderThemeChromiumSkia::focusRingColor() const
+Color RenderThemeChromiumSkia::platformFocusRingColor() const
 {
     static Color focusRingColor(229, 151, 0, 255);

trunk/WebCore/rendering/RenderThemeChromiumSkia.h

@@ -55 +55 @@
         virtual Color platformActiveSelectionForegroundColor() const;
         virtual Color platformInactiveSelectionForegroundColor() const;
-        virtual Color focusRingColor() const;
+        virtual Color platformFocusRingColor() const;
 
         // To change the blink interval, override caretBlinkIntervalInternal instead of this one so that we may share layout test code an intercepts.

trunk/WebCore/rendering/RenderThemeMac.h

@@ -59 +59 @@
     virtual Color platformInactiveListBoxSelectionBackgroundColor() const;
     virtual Color platformInactiveListBoxSelectionForegroundColor() const;
-    virtual Color focusRingColor() const;
+    virtual Color platformFocusRingColor() const;
 
     virtual ScrollbarControlSize scrollbarControlSizeForPart(ControlPart) { return SmallScrollbar; }

trunk/WebCore/rendering/RenderThemeMac.mm

@@ -158 +158 @@
 }
 
-Color RenderThemeMac::focusRingColor() const
+Color RenderThemeMac::platformFocusRingColor() const
 {
     if (usesTestModeFocusRingColor())

trunk/WebCore/rendering/RenderThemeSafari.cpp

@@ -80 +80 @@
     // potentially wrong answer the very first time it's called (see
     // <https://bugs.webkit.org/show_bug.cgi?id=26493>).
-    if (Settings::shouldPaintNativeControls())
+    if (Settings::shouldPaintNativeControls()) {
+        RenderTheme::setCustomFocusRingColor(safariTheme->platformFocusRingColor());
         return windowsTheme; // keep the reference of one.
+    }
     return safariTheme; // keep the reference of one.
 }
@@ -161 +163 @@
 }
 
-Color RenderThemeSafari::focusRingColor() const
+Color RenderThemeSafari::platformFocusRingColor() const
 {
     static Color focusRingColor;

trunk/WebCore/rendering/RenderThemeSafari.h

@@ -70 +70 @@
     virtual Color activeListBoxSelectionBackgroundColor() const;
 
-    virtual Color focusRingColor() const;
+    virtual Color platformFocusRingColor() const;
 
     // System fonts.