Bug 148758

Summary: Need to be able to test default behaviors on force click
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, buildbot, commit-queue, rniwa, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-mavericks
none
Patch thorton: review+

Description Beth Dakin 2015-09-03 16:33:21 PDT
Need to be able to test default behaviors on force click
Comment 1 Beth Dakin 2015-09-03 16:45:19 PDT
Created attachment 260539 [details]
Patch
Comment 2 WebKit Commit Bot 2015-09-03 16:47:38 PDT
Attachment 260539 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:376:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:377:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:378:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:380:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:381:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:382:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:438:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:439:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:440:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:442:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:443:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitTestRunner/mac/EventSenderProxy.mm:444:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 12 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Radar WebKit Bug Importer 2015-09-03 17:49:36 PDT
<rdar://problem/22569900>
Comment 4 Tim Horton 2015-09-03 17:58:34 PDT
Comment on attachment 260539 [details]
Patch

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

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:102
> +    _eventSender_subtype = NSWindowExposedEventType;

This is zero, so I think you should just omit it (I don't think this is really meant to be NSWindowExposedEventType, it just happens to be the same as 0).

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:374
> +    if ([m_testController->mainWebView()->platformView() hitTest:[event locationInWindow]])

Why bother with the hit test? We don't use its result, and don't need to do it.
Comment 5 Beth Dakin 2015-09-03 22:02:47 PDT
(In reply to comment #4)
> Comment on attachment 260539 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=260539&action=review
> 
> > Tools/WebKitTestRunner/mac/EventSenderProxy.mm:102
> > +    _eventSender_subtype = NSWindowExposedEventType;
> 
> This is zero, so I think you should just omit it (I don't think this is
> really meant to be NSWindowExposedEventType, it just happens to be the same
> as 0).

You're right! I don't seem to need it.

> 
> > Tools/WebKitTestRunner/mac/EventSenderProxy.mm:374
> > +    if ([m_testController->mainWebView()->platformView() hitTest:[event locationInWindow]])
> 
> Why bother with the hit test? We don't use its result, and don't need to do
> it.

Good catch! I removed it.
Comment 6 Beth Dakin 2015-09-03 22:08:32 PDT
Created attachment 260566 [details]
Patch
Comment 7 Beth Dakin 2015-09-04 10:12:41 PDT
Created attachment 260594 [details]
Patch
Comment 8 Build Bot 2015-09-04 10:46:02 PDT
Comment on attachment 260594 [details]
Patch

Attachment 260594 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/140258

New failing tests:
fast/events/force-click-on-link-navigation.html
Comment 9 Build Bot 2015-09-04 10:46:05 PDT
Created attachment 260596 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 10 Beth Dakin 2015-09-04 11:20:39 PDT
Created attachment 260600 [details]
Patch
Comment 11 Tim Horton 2015-09-04 11:31:50 PDT
Comment on attachment 260600 [details]
Patch

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

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:95
> +    _eventSender_momentumPhase = 0;

No need for this since it's ObjC

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:376
> +static void spinRunLoopForForce()

handleNextPressureEvent() or something! we're not spinning (just handling the next enqueued pressure event), and we should probably be explicit about which event type we're handling.

Actually, since you always -_postDelayed and then spinRunLoopForForce(), you could make it handleForceEventSynchronously(NSEvent *) and do both things inside here! and that will make the other code less repeaty
Comment 12 Beth Dakin 2015-09-04 11:46:31 PDT
http://trac.webkit.org/changeset/189365