Bug 66173 - [Qt] Add eventSender.gestureTap
Summary: [Qt] Add eventSender.gestureTap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on: 79359 92895
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-12 15:40 PDT by Ryosuke Niwa
Modified: 2012-10-22 04:54 PDT (History)
7 users (show)

See Also:


Attachments
fix (1.87 KB, patch)
2011-08-12 15:46 PDT, Sadrul Habib Chowdhury
no flags Details | Formatted Diff | Diff
Patch (3.03 KB, patch)
2012-02-22 10:10 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (4.86 KB, patch)
2012-09-19 06:31 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch2 (12.80 KB, patch)
2012-09-21 08:02 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-08-12 15:40:21 PDT
r92997 added new eventSender method: gestureTap.  Qt should implement this feature.
Comment 1 Sadrul Habib Chowdhury 2011-08-12 15:46:09 PDT
Created attachment 103830 [details]
fix
Comment 2 Sadrul Habib Chowdhury 2011-08-12 15:46:58 PDT
The fast/event/touch tests are already skipped in other platforms (e.g. win, mac, gtk)
Comment 3 Ryosuke Niwa 2011-08-12 15:47:37 PDT
Comment on attachment 103830 [details]
fix

I'm adding the test to the Qt's skipped list for now.  This patch isn't going to fix this bug because it doesn't actually implement the eventSender method.
Comment 4 Ryosuke Niwa 2011-08-12 15:50:17 PDT
The test skipped in http://trac.webkit.org/changeset/93006.
Comment 5 Allan Sandfeld Jensen 2012-02-22 10:10:24 PST
Created attachment 128237 [details]
Patch
Comment 6 WebKit Review Bot 2012-02-22 16:55:56 PST
Comment on attachment 128237 [details]
Patch

Clearing flags on attachment: 128237

Committed r108577: <http://trac.webkit.org/changeset/108577>
Comment 7 WebKit Review Bot 2012-02-22 16:56:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Allan Sandfeld Jensen 2012-02-23 07:04:11 PST
The patch did not implement the tap gesture in a functional way, and I am not sure we want to implement it since it is completely absent from the WebKitTestRunner.
Comment 9 Allan Sandfeld Jensen 2012-09-19 06:31:05 PDT
Created attachment 164729 [details]
Patch
Comment 10 WebKit Review Bot 2012-09-20 01:19:09 PDT
Comment on attachment 164729 [details]
Patch

Clearing flags on attachment: 164729

Committed r129102: <http://trac.webkit.org/changeset/129102>
Comment 11 WebKit Review Bot 2012-09-20 01:19:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Allan Sandfeld Jensen 2012-09-21 07:58:23 PDT
The implemenatation lacked handling of the GestureEvent in WebKit code. I missed this for the second time, because the default expected result of the test-case is actually a fail.
Comment 13 Allan Sandfeld Jensen 2012-09-21 08:02:03 PDT
Created attachment 165132 [details]
Patch2
Comment 14 WebKit Review Bot 2012-09-21 08:05:46 PDT
Attachment 165132 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebKit/qt/Api/qwebpage.cpp:98:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/qt/Api/qwebpage.cpp:136:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Simon Hausmann 2012-10-21 23:39:52 PDT
Comment on attachment 165132 [details]
Patch2

View in context: https://bugs.webkit.org/attachment.cgi?id=165132&action=review

r=me but needs #include order fix before landing I suppose :)

> Source/WebKit/qt/WebCoreSupport/WebEventConversion.cpp:322
> +        m_position = event->widget()->mapFromGlobal(globalPos.toPoint());

With the upcoming QtWebKit <> QtWebKitWidgets split in mind it would be great if the widget local position could be provided as a parameter from outside to reduce the amount of widget code here :)
Comment 16 Allan Sandfeld Jensen 2012-10-22 02:38:25 PDT
(In reply to comment #15)
> (From update of attachment 165132 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165132&action=review
> 
> r=me but needs #include order fix before landing I suppose :)
> 
> > Source/WebKit/qt/WebCoreSupport/WebEventConversion.cpp:322
> > +        m_position = event->widget()->mapFromGlobal(globalPos.toPoint());
> 
> With the upcoming QtWebKit <> QtWebKitWidgets split in mind it would be great if the widget local position could be provided as a parameter from outside to reduce the amount of widget code here :)

But wouldn't QtWebKit(1) be in QtWebKitWidgets, and therefore allowed to depend on Widget code?
Comment 17 Allan Sandfeld Jensen 2012-10-22 04:53:59 PDT
Committed in r132052
Comment 18 Allan Sandfeld Jensen 2012-10-22 04:54:20 PDT
Comment on attachment 165132 [details]
Patch2

Clearing flags