Changeset 52664 in webkit

Timestamp:

Dec 30, 2009 9:42:05 AM (14 years ago)

Author:

eric@webkit.org

Message:

2009-12-30 Carol Szabo <​carol.szabo@nokia.com>

Reviewed by Laszlo Gombos.

[Qt] RenderThemeQt::applyTheme is a misnomer and is suboptimally coded.

​https://bugs.webkit.org/show_bug.cgi?id=33035

This patch:

• renames RenderThemeQt::applyTheme to initializeCommonQStyleOptions,
• extracts the palette initialization code to a separate function in order to provide for readable pointer checking and moves this code up in the function to allow for future changes to the palette brushes needed for bug 30173,
• optimizes some of the code in the function for readability, speed and size.
• fixes some minor style issues

No new tests because code behavior is not changed.

• platform/qt/RenderThemeQt.cpp: (WebCore::RenderThemeQt::paintButton): (WebCore::RenderThemeQt::paintTextField): (WebCore::RenderThemeQt::paintMenuList): (WebCore::RenderThemeQt::paintMenuListButton): (WebCore::initPaletteFromPageClientIfExists): (WebCore::RenderThemeQt::initializeCommonQStyleOptions):
• platform/qt/RenderThemeQt.h:

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/qt/RenderThemeQt.cpp (modified) (8 diffs)
• platform/qt/RenderThemeQt.h (modified) (3 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-12-30  Carol Szabo  <carol.szabo@nokia.com>
+
+        Reviewed by Laszlo Gombos.
+
+         [Qt] RenderThemeQt::applyTheme is a misnomer and is suboptimally coded.
+        https://bugs.webkit.org/show_bug.cgi?id=33035
+
+        This patch:
+        - renames RenderThemeQt::applyTheme to initializeCommonQStyleOptions,
+        - extracts the palette initialization code to a separate function in order to
+        provide for readable pointer checking and moves this code up in the function to
+        allow for future changes to the palette brushes needed for bug 30173,
+        - optimizes some of the code in the function for readability, speed and size.
+        - fixes some minor style issues
+
+        No new tests because code behavior is not changed.
+
+        * platform/qt/RenderThemeQt.cpp:
+        (WebCore::RenderThemeQt::paintButton):
+        (WebCore::RenderThemeQt::paintTextField):
+        (WebCore::RenderThemeQt::paintMenuList):
+        (WebCore::RenderThemeQt::paintMenuListButton):
+        (WebCore::initPaletteFromPageClientIfExists):
+        (WebCore::RenderThemeQt::initializeCommonQStyleOptions):
+        * platform/qt/RenderThemeQt.h:
+
 2009-12-30  Laszlo Gombos  <laszlo.1.gombos@nokia.com>
 

trunk/WebCore/platform/qt/RenderThemeQt.cpp

@@ -44 +44 @@
 #include "NotImplemented.h"
 #include "Page.h"
+#include "QWebPageClient.h"
 #include "RenderBox.h"
 #include "RenderTheme.h"
 #include "UserAgentStyleSheets.h"
-#include "QWebPageClient.h"
 #include "qwebpage.h"
 
@@ -478 +478 @@
     option.state |= QStyle::State_Small;
 
-    ControlPart appearance = applyTheme(option, o);
+    ControlPart appearance = initializeCommonQStyleOptions(option, o);
     if (appearance == PushButtonPart || appearance == ButtonPart) {
         option.rect = inflateButtonRect(option.rect, qStyle());
@@ -514 +514 @@
 
     // Get the correct theme data for a text field
-    ControlPart appearance = applyTheme(panel, o);
+    ControlPart appearance = initializeCommonQStyleOptions(panel, o);
     if (appearance != TextFieldPart
         && appearance != SearchFieldPart
@@ -576 +576 @@
     if (p.widget)
         opt.initFrom(p.widget);
-    applyTheme(opt, o);
+    initializeCommonQStyleOptions(opt, o);
 
     const QPoint topLeft = r.topLeft();
@@ -616 +616 @@
     if (p.widget)
         option.initFrom(p.widget);
-    applyTheme(option, o);
+    initializeCommonQStyleOptions(option, o);
     option.rect = r;
 
@@ -713 +713 @@
 }
 
-ControlPart RenderThemeQt::applyTheme(QStyleOption& option, RenderObject* o) const
+static inline void setPaletteFromPageClientIfExists(QPalette &palette, const RenderObject *o)
+{
+    // If the webview has a custom palette, use it
+    Page* page = o->document()->page();
+    if (!page)
+        return;
+    Chrome* chrome = page->chrome();
+    if (!chrome)
+        return;
+    ChromeClient* chromeClient = chrome->client();
+    if (!chromeClient)
+        return;
+    QWebPageClient* pageClient = chromeClient->platformPageClient();
+    if (!pageClient)
+        return;
+    palette = pageClient->palette();
+}
+
+ControlPart RenderThemeQt::initializeCommonQStyleOptions(QStyleOption& option, RenderObject* o) const
 {
     // Default bits: no focus, no mouse over
@@ -725 +743 @@
         option.state |= QStyle::State_ReadOnly;
 
-    if (supportsFocus(o->style()->appearance()) && isFocused(o)) {
+    option.direction = Qt::LeftToRight;
+
+    if (isHovered(o))
+        option.state |= QStyle::State_MouseOver;
+
+    setPaletteFromPageClientIfExists(option.palette, o);
+    RenderStyle* style = o->style();
+    if (!style)
+        return NoControlPart;
+
+    ControlPart result = style->appearance();
+    if (supportsFocus(result) && isFocused(o)) {
         option.state |= QStyle::State_HasFocus;
         option.state |= QStyle::State_KeyboardFocusChange;
     }
 
-    if (isHovered(o))
-        option.state |= QStyle::State_MouseOver;
-
-    option.direction = Qt::LeftToRight;
-    if (o->style() && o->style()->direction() == WebCore::RTL)
+    if (style->direction() == WebCore::RTL)
         option.direction = Qt::RightToLeft;
-
-    ControlPart result = o->style()->appearance();
 
     switch (result) {
@@ -754 +777 @@
         break;
     }
-    }
-
-    if (result == RadioPart || result == CheckboxPart)
+    case RadioPart:
+    case CheckboxPart:
         option.state |= (isChecked(o) ? QStyle::State_On : QStyle::State_Off);
-
-    // If the owner widget has a custom palette, use it
-    Page* page = o->document()->page();
-    if (page) {
-        ChromeClient* client = page->chrome()->client();
-        QWebPageClient* pageClient = client->platformPageClient();
-        if (pageClient)
-            option.palette = pageClient->palette();
     }
 

trunk/WebCore/platform/qt/RenderThemeQt.h

@@ -20 +20 @@
  *
  */
-#ifndef RenderThemeQt_H
-#define RenderThemeQt_H
+#ifndef RenderThemeQt_h
+#define RenderThemeQt_h
 
 #include "RenderTheme.h"
@@ -133 +133 @@
     bool supportsFocus(ControlPart) const;
 
-    ControlPart applyTheme(QStyleOption&, RenderObject*) const;
+    ControlPart initializeCommonQStyleOptions(QStyleOption&, RenderObject*) const;
 
     void setButtonPadding(RenderStyle*) const;
@@ -181 +181 @@
 }
 
-#endif
+#endif // RenderThemeQt_h