[Webkit-unassigned] [Bug 15737] [GTK] A MenuList should look like a GtkComboBox

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=15737


aroben at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16971|review?                     |review+
               Flag|                            |




------- Comment #8 from aroben at apple.com  2007-11-02 21:46 PDT -------
(From update of attachment 16971)
It looks like in many of the adjust*Style methods you are no longer allowing
web developers to style form controls at all, where the old code allowed for at
least limited styling. Are you sure this is the direction the GTK port should
go in? In the Mac/Windows ports there are heuristics to determine whether the
native look should be used or a more generic, styled look should be used for
each control, which gives a nice balance between native look-and-feel and
stylability. Maybe that's something you want to emulate?

+    // TODO: This strategy is possibly incorrect for the GTK+ port.

Would you mind changing all your TODOs to FIXMEs?

+static void setMozState(RenderTheme* theme, GtkWidgetState* state,
RenderObject* o)

It would be slightly nicer for the second paramater to be a GtkWidgetState&,
but it's not a big deal.
+    //state->active = theme->isChecked(o);

I think this commented-out code should be removed.

+    // It could be made a configuration option if 13 actually breaks site
compatibility.

I think you mean "...if sizes other than 13 actually break..."

+void RenderThemeGtk::adjustSearchFieldResultsButtonStyle(CSSStyleSelector*
selector, RenderStyle* style, Element* e) const
+{
+    adjustSearchFieldCancelButtonStyle(selector, style, e);
+}

This seems a little strange to me, but I suppose you're just saying that the
cancel and results buttons should have the same style? It's a little confusing,
though, since adjustSearchFieldResultsDecorationStyle doesn't call through to
adjustSearchFieldCancelButtonStyle.

+bool RenderThemeGtk::paintSearchFieldResultsDecoration(RenderObject* o, const
RenderObject::PaintInfo& i, const IntRect& rect)
+{
+    return paintSearchFieldCancelButton(o, i, rect);
+}

I'm very confused by this. On Mac/Windows, the results button and the results
decoration are both magnifying glasses. The difference is that the results
button has a little downward-pointing arrow that indicates it will show a menu
when you click on it. I don't think you want to show a cancel button where the
results decoration goes, because then you'd have cancel buttons on both sides
of the control, but one wouldn't do anything.

+static void gtkStyleSetCb(GtkWidget* widget, GtkStyle* previous, RenderTheme*
renderTheme)

Can you replace "Cb" with "Callback"?

r=me, though I do think you should consider the issue of stylability. You could
land the code as-is and file a bug about it.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.