[Webkit-unassigned] [Bug 29564] [Qt] Webkit Button Size

[bugzilla-daemon at webkit.org]

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


Tor Arne Vestbø <vestbo at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #46689|                            |review-
               Flag|                            |




--- Comment #13 from Tor Arne Vestbø <vestbo at webkit.org>  2010-01-18 04:26:54 PST ---
(From update of attachment 46689)
Thanks for the updated patch Daniel!

> +#include <QMacStyle>

QMacStyle is only available on mac, so this has to be wrapped in Q_WS_MAC

>      QFont defaultButtonFont = QApplication::font(&button);
>      QFontInfo fontInfo(defaultButtonFont);
>      m_buttonFontFamily = defaultButtonFont.family();
> -#ifdef Q_WS_MAC
> -    m_buttonFontPixelSize = fontInfo.pixelSize();
> -#endif
> +    if (qobject_cast<QMacStyle*>(qStyle()))
> +        m_buttonFontPixelSize = fontInfo.pixelSize();
>  }

We can't reference the style in the ctor like this, it's too early. You could
move the init code into adjustButtonSizes() where those two members are used
and use static variables there to cache it.

the QMacStyle check also has to be wrapped in Q_WS_MAC like the include

> +    if (qobject_cast<QMacStyle*>(qStyle()) && style->appearance() == PushButtonPart) {

Same

> +    if (qobject_cast<QMacStyle*>(qStyle()) {

Same


> Index: LayoutTests/fast/css/button-height-expected.txt
> ===================================================================
> --- LayoutTests/fast/css/button-height-expected.txt	(revision 0)
> +++ LayoutTests/fast/css/button-height-expected.txt	(revision 0)
> @@ -0,0 +1,13 @@

This is a cross-platform expected file, which means it must be valid for mac,
win, qt, gtk, qt-mac, etc if there are no overrides in platform/port-spesific
results.

> +PASS document.getElementById('button1').offsetHeight is document.getElementById('button2').offsetHeight
> +PASS document.getElementById('button3').offsetHeight is 40
> +PASS document.getElementById('button4').offsetHeight is 40
> +PASS document.getElementById('button5').offsetHeight is 18

The hard-coded result of 18 here will not match for example qt, gtk, where the
button will have a height of 40.

> +    shouldEvaluateTo("document.getElementById('button5').offsetHeight", 
> +        (navigator.platform.indexOf("Mac") !== -1) ? document.getElementById("button1").offsetHeight : 40);

This one is tricky. For Qt we run the DRT with -style windows on all platforms,
which means that with the runtime check in RenderThemeQt we'll render the
button 40 pixel high, but navigator.platform will still return MacIntel, so
we'll expect the height to be button1.

I guess the easiest here would be to change this line into dumping the height
of the button5, and then using the expected-file to detect failure, with the
cross-platform expected file expecting 40, and then a platform-specific
expected file with 18 in the mac results.

(or a number of platform-specific expected files with 40 instead of a shared
cross-platform one) + the mac one with 18

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