Bug 158601 – Add support for passive event listeners on touch events

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

Add support for passive event listeners on touch events

[Benjamin Poulain]

Created attachment 280982 [details]
Patch

[WebKit Commit Bot]

Attachment 280982 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:488:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 1 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

\o/

[Build Bot]

Comment on attachment 280982 [details]
Patch

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

New failing tests:
tiled-drawing/scrolling/frames/scroll-region-after-frame-layout.html
tiled-drawing/scrolling/frames/coordinated-frame.html
tiled-drawing/scrolling/frames/coordinated-frame-lose-scrolling-ancestor.html
tiled-drawing/scrolling/frames/coordinated-frame-in-fixed.html
tiled-drawing/scrolling/fixed/fixed-in-overflow.html
tiled-drawing/scrolling/frames/coordinated-frame-gain-scrolling-ancestor.html

[Build Bot]

Created attachment 280987 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Benjamin Poulain]

I'll check the WK2 tests tomorrow. Piano tonight.

[Rick Byers]

This is awesome, thanks for working on it Ben!

One suggestion.  It looks like you're treating all touch event types the same, right?  One common scenario is to have passive touchstart and touchmove listeners but a non-passive touchend listener (so you can still prevent the generation of mouse/click events and associated behavior - eg. as is common today with fastclick libraries as much I want those to disappear).  In blink we were careful to ensure that case still didn't block scrolling (http://crbug.com/601990).  You can see this at http://rbyers.net/scroll-latency.html - set the listener to touchend, enable showing non-blocking events, and tap.  You'll get an uncancelable touchstart but a cancelable touchend event.  Basically we treat touchstart, touchmove and wheel/mousewheel specially as the only "scroll blocking events".

That's just a performance optimization so I don't think interoperability here matters much (although maybe it would be nice to spec the precise behavior here and add some web-platform-tests if we end up matching).  But figured I'd mention it in case it matters to you.

[Simon Fraser (smfr)]

Comment on attachment 280982 [details]
Patch

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

> Source/WebCore/ChangeLog:32
> +        If a Target has multiple listener for an event type, we want to know

listeners

> Source/WebCore/dom/EventTarget.cpp:252
> +    return hasActiveEventListeners(names.touchstartEvent)
> +        || hasActiveEventListeners(names.touchmoveEvent)
> +        || hasActiveEventListeners(names.touchendEvent)
> +        || hasActiveEventListeners(names.touchcancelEvent)
> +        || hasActiveEventListeners(names.touchforcechangeEvent);

Maybe adjust this given Rick's comment.

> Source/WebCore/page/DebugPageOverlays.cpp:137
> +        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator()) {
> +            EventTrackingRegions eventTrackingRegions = scrollingCoordinator->absoluteEventTrackingRegions();
> +            *region = eventTrackingRegions.synchronousDispatchRegion;
> +        }

We could fix the overlay to show both regions.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1994
> +        // But, here we know that every events handlers that can handle this events are passive.

"every events handlers" -> "all events handlers"

[Benjamin Poulain]

(In reply to comment #7)
> One suggestion.  It looks like you're treating all touch event types the
> same, right?  One common scenario is to have passive touchstart and
> touchmove listeners but a non-passive touchend listener (so you can still
> prevent the generation of mouse/click events and associated behavior - eg.
> as is common today with fastclick libraries as much I want those to
> disappear).  In blink we were careful to ensure that case still didn't block
> scrolling (http://crbug.com/601990).  You can see this at
> http://rbyers.net/scroll-latency.html - set the listener to touchend, enable
> showing non-blocking events, and tap.  You'll get an uncancelable touchstart
> but a cancelable touchend event.  Basically we treat touchstart, touchmove
> and wheel/mousewheel specially as the only "scroll blocking events".

That's a very good point.

[Benjamin Poulain]

Created attachment 281044 [details]
Patch

[WebKit Commit Bot]

Attachment 281044 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:488:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 1 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Benjamin Poulain]

Comment on attachment 281044 [details]
Patch

Clearing flags on attachment: 281044

Committed r201958: <http://trac.webkit.org/changeset/201958>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.