RenderThemeGdk's buttons are state-agnostic (pressed, hovered)

[Nigel Tao]

On the GTK platform (built using "./build-webkit --gdk") the buttons don't look any different when pressed, or when hovered over, even though the GTK theme supports this.

[Nigel Tao]

Created attachment 15618 [details]
Simple Patch

[Adam Roben (:aroben)]

Comment on attachment 15618 [details]
Simple Patch

+    GtkStateType state_type = isHovered(o)
+        ? (isPressed(o) ? GTK_STATE_ACTIVE : GTK_STATE_PRELIGHT)
+        : GTK_STATE_NORMAL;

I think this line would be clearer as:

GtkStateType state_type = isPressed(o) ? GTK_STATE_ACTIVE : (isHovered(o) ? GTK_STATE_PRELIGHT : GTK_STATE_NORMAL);

...or...

GtkStateType state_type = GTK_STATE_NORMAL;
if (isPressed(o))
    state_type = GTK_STATE_ACTIVE;
else if (isHovered(o))
    state_type = GTK_STATE_PRELIGHT;

I personally prefer the second one.

Also, the variable name sould be `stateType` to adhere to our style guidelines <http://webkit.org/coding/coding-style.html>

[Nigel Tao]

Created attachment 15647 [details]
Better Simple Patch

[Alp Toker]

Created attachment 15666 [details]
Make use of widget state in RenderThemeGdk

[Alp Toker]

Created attachment 15667 [details]
Make use of widget state in RenderThemeGdk

Indentation corrected in this revision.

[Adam Roben (:aroben)]

Comment on attachment 15666 [details]
Make use of widget state in RenderThemeGdk

isChecked(o) ? GTK_SHADOW_IN : GTK_SHADOW_OUT

Perhaps this code should become another helper function called `checkedState` (or shadowState if that makes more sense in Gtk terminology)

r=me

[Adam Roben (:aroben)]

Comment on attachment 15667 [details]
Make use of widget state in RenderThemeGdk

r=me to this one too, same comments as above.

[Alp Toker]

Fixed in r24595.

[Nigel Tao]

It looks like Alp Toker's patch has landed in trunk, but the pushbuttons' backgrounds are still being (incorrectly) filled in.  You can see the problem in the screenshot at http://blogs.gnome.org/nigeltao/files/2007/07/gdk-webkit-buttons.png

You might want to apply this part of my patch:

Index: WebCore/platform/gdk/RenderThemeGdk.cpp
===================================================================
--- WebCore/platform/gdk/RenderThemeGdk.cpp     (revision 24641)
+++ WebCore/platform/gdk/RenderThemeGdk.cpp     (working copy)
@@ -288,7 +288,13 @@
 {
     if (!m_container) {
         m_unmappedWindow = gtk_window_new(GTK_WINDOW_POPUP);
-        m_container = gtk_fixed_new();
+        // Some GTK themes (i.e. Clearlooks) draw the buttons differently
+        // (in particular, call gtk_style_apply_default_background) if they
+        // are unallocated and are children of a GtkFixed widget, which is
+        // apparently what some "make Firefox buttons look like GTK" code
+        // does.  To avoid this ugly look, we use a GtkHBox as a parent,
+        // rather than a GtkFixed.
+        m_container = gtk_hbox_new(false, 0);
         gtk_container_add(GTK_CONTAINER(m_unmappedWindow), m_container);
         gtk_widget_realize(m_unmappedWindow);
     }

[Alp Toker]

Created attachment 15721 [details]
Follow-up for shadow state and theming bug workaround

[Alp Toker]

Created attachment 15722 [details]
Follow-up for shadow state and theming bug workaround (take two)

Remove the old code we're replacing this time.

[Mark Rowe (bdash)]

Comment on attachment 15722 [details]
Follow-up for shadow state and theming bug workaround (take two)

r=me