[Webkit-unassigned] [Bug 19069] Renderers for wx

[bugzilla-daemon at webkit.org]

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


ddkilzer at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #21153|review?(kevino at theolliviers.|review-
               Flag|com)                        |




------- Comment #2 from ddkilzer at webkit.org  2008-05-28 07:55 PDT -------
(From update of attachment 21153)
>+#include "WebKit/wx/WebView.h"
>+
> #include <wx/defs.h>
> #include <wx/renderer.h>
> #include <wx/dcclient.h>

Should this be #include <wx/WebView.h>?

>+void RenderThemeWx::adjustRepaintRect(const RenderObject* o, IntRect& r)
>+{
>+    switch (o->style()->appearance()) {
>+        case MenulistAppearance: {
>+            r.setWidth(r.width() + 100); // = inflateRect(r, popupButtonSizes()[[popupButton() controlSize]], popupButtonMargins());
>+            //fprintf(stderr, "caling adjustRepaintRect, width is %d...\n", r.width());
>+            break;
>+        }
>+        default:
>+            break;
>+    }
>+}

Please remove commented-out code (especially debugging statements :).

>     if (!isEnabled(o))
>         flags |= wxCONTROL_DISABLED;
>-
>+        
>     EAppearance appearance = o->style()->appearance();
>     if (supportsFocus(o->style()->appearance()) && isFocused(o))
>         flags |= wxCONTROL_FOCUSED;
>@@ -204,16 +233,18 @@
>     if (isPressed(o))
>         flags |= wxCONTROL_PRESSED;
>     
>-    if (appearance == PushButtonAppearance || appearance == ButtonAppearance)
>+    if (appearance == PushButtonAppearance || appearance == ButtonAppearance) 
>         wxRendererNative::Get().DrawPushButton(window, *dc, r, flags);

Gratuitous whitespace changes.

>+int RenderThemeWx::popupInternalPaddingLeft(RenderStyle*) const 
>+{ 
>+    return 6; 
> }
> 
>+int RenderThemeWx::popupInternalPaddingRight(RenderStyle*) const 
>+{
>+#ifdef __WXMAC__
>+    return 22;
>+#else
>+    return 20;
>+#endif
>+}
>+
>+int RenderThemeWx::popupInternalPaddingTop(RenderStyle*) const 
>+{
>+    return 2;
>+}
>+
>+int RenderThemeWx::popupInternalPaddingBottom(RenderStyle*) const
>+{ 
>+    return 2; 
>+}

Might be nice to use const int variables (to match RenderThemeMac.mm and
RenderThemeSafari.cpp), but not necessary.

Needs a ChangeLog entry (as do your other patches :).  See
<http://webkit.org/coding/contributing.html> for details.

r- for commented-out code, #include formatting and no ChangeLog entry, but
otherwise the patch looks good!  Thanks, Robin!


-- 
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.