[webkit-reviews] review denied: [Bug 46039] [Qt] QtTestBrowser: Make nodesFromRect testable. : [Attachment 68012] patch v1 (it will fail to build until 44088 and 44089 land)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 6 20:49:39 PDT 2010
Andreas Kling <kling at webkit.org> has denied Antonio Gomes
<tonikitoo at webkit.org>'s request for review:
Bug 46039: [Qt] QtTestBrowser: Make nodesFromRect testable.
https://bugs.webkit.org/show_bug.cgi?id=46039
Attachment 68012: patch v1 (it will fail to build until 44088 and 44089 land)
https://bugs.webkit.org/attachment.cgi?id=68012&action=review
------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68012&action=review
> WebKitTools/QtTestBrowser/launcherwindow.cpp:88
> + // Inspector set up.
setup
> WebKitTools/QtTestBrowser/launcherwindow.cpp:98
> + // Zooming set up.
setup
> WebKitTools/QtTestBrowser/launcherwindow.cpp:358
> + paddingNone->setChecked((m_windowOptions.tapPaddingArea ==
gTapPaddingAreaNone) ? true : false);
"? true : false" is redundant.
> WebKitTools/QtTestBrowser/launcherwindow.cpp:362
> + padding10x10->setChecked((m_windowOptions.tapPaddingArea ==
gTapPaddingArea10x10) ? true : false);
Ditto.
> WebKitTools/QtTestBrowser/launcherwindow.cpp:366
> + padding10x15->setChecked((m_windowOptions.tapPaddingArea ==
gTapPaddingArea10x15) ? true : false);
Ditto.
> WebKitTools/QtTestBrowser/launcherwindow.cpp:829
> + QStringList size = padding.split("x");
I don't particularly like the name "size" for a string list.
Also, nit: padding.split('x') rather than padding.split("x")
> WebKitTools/QtTestBrowser/webview.cpp:249
> + case QEvent::GraphicsSceneHoverLeave:
> + if (m_showTapPaddingArea)
> + moveMouseCursorOffscreen();
> + default:
Missing break statement.
More information about the webkit-reviews
mailing list