Bug 46039 - [Qt] QtTestBrowser: Make nodesFromRect testable.
Summary: [Qt] QtTestBrowser: Make nodesFromRect testable.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Antonio Gomes
URL:
Keywords: Qt
Depends on: 44088 44089
Blocks: 40477
  Show dependency treegraph
 
Reported: 2010-09-18 09:30 PDT by Antonio Gomes
Modified: 2012-04-12 11:43 PDT (History)
4 users (show)

See Also:


Attachments
patch v1 (it will fail to build until 44088 and 44089 land) (11.77 KB, patch)
2010-09-18 09:34 PDT, Antonio Gomes
kling: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2010-09-18 09:30:08 PDT
Bug 44088 and bug 44089 are adding to QtWebKit and ability to make use of Document::nodesFromRect() to improve the touch experience on mobile devices.

To make it easier to validate the implementation of both bugs, I added testing support for it to the QtTestBrowser.
Comment 1 Antonio Gomes 2010-09-18 09:34:45 PDT
Created attachment 68012 [details]
patch v1 (it will fail to build until 44088 and 44089 land)
Comment 2 Early Warning System Bot 2010-09-18 17:31:31 PDT
Attachment 68012 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4027062
Comment 3 Antonio Gomes 2010-09-18 17:39:57 PDT
(In reply to comment #2)
> Attachment 68012 [details] did not build on qt:
> Build output: http://queues.webkit.org/results/4027062

Expected
Comment 4 Andreas Kling 2010-10-06 20:49:40 PDT
Comment on attachment 68012 [details]
patch v1 (it will fail to build until 44088 and 44089 land)

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.