[webkit-reviews] review denied: [Bug 31742] [Qt] QWebView ignores a palette set with QWebView::setPalette() : [Attachment 81390] more tab removals

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin.poulain at nokia.com> has denied Fabrizio
<fabrizio.machado at nokia.com>'s request for review:
Bug 31742: [Qt] QWebView ignores a palette set with QWebView::setPalette()
https://bugs.webkit.org/show_bug.cgi?id=31742

Attachment 81390: more tab removals
https://bugs.webkit.org/attachment.cgi?id=81390&action=review

------- Additional Comments from Benjamin Poulain <benjamin.poulain at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=81390&action=review

The test needs some rework to make it simpler and test one feature at the time.


> Source/WebCore/platform/qt/RenderThemeQt.cpp:1435
>  {
> -    return  QApplication::cursorFlashTime() / 1000.0 / 2.0;
> +    return QApplication::cursorFlashTime() / 1000.0 / 2.0;
>  }

This is unrelated. You should a separate patch for this.

> Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp:34
> -{
> +class tst_QWebView : public QObject {

You should fix the style in a separate patch.

> Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp:325
> +    // Test 1:  BEGIN:  Selection background color on active QWebView   

You should split the test in multiple, simpler functions. Each testing one
aspect of the patch.

> Source/WebKit/qt/tests/qwebview/tst_qwebview.cpp:349
> +    QString strActiveRedBg="activeRedBg.png";
> +    imgActiveRedBg.save(strActiveRedBg);

Why do you save the image in a test?