Bug 184250 – Unable to call event.preventDefault in dynamically added event listener

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

Hi there,

When you add an event listener (the parent listener) and you add another event listener inside of it (the child listener), the dynamically added child event listener is not able to use event.preventDefault() as the event is non cancelable. The dynamically added child listener will receive the event - but it is unable to use preventDefault(). This is true regardless of whether the parent event handler is a passive or capture handler (or both).

This is true for all the event types I tested.

Here a simple example that reproduces the issue:

- desktop: https://codepen.io/alexreardon/pen/eMKMmm
- mobile: https://codepen.io/alexreardon/pen/yKEKby

Able to correctly use event.preventDefault() in child listener on:
- Chrome
- Firefox
- Edge
- Internet Explorer 11
- Android: Chrome mobile 
- Android: Firefox (which I think just uses chrome under the hood)
- Safari 10.x (I was unable to test 11.2 as I could not find 

Not able to use event.preventDefault() in child listener on:
- Safari 11.3 on mac 
- Safari 11.3 on iOS 11.3 (as well as the other browsers that use the native browser such as firefox and chrome)
- Safari on iOS 11.2.6 (it looks like there was a configuration where event.preventDefault() was working. I think it had to do with touch-action: manipulation but I am yet to confirm)

This is important to us because we use event.preventDefault() on a touchmove event to prevent native scrolling on mobile. I can 

Work around:

You need to add an event handler in a non dynamic way (ie at startup) and then you can use event.preventDefault() as expected.

Impact:

A lot of drag and drop libraries and other tools use event.preventDefault() on touchmove to opt out of native scrolling. Right now this is not possible without adding your own touchmove listener before a touchstart

A bit of context, upgrading from iOS 11.2 to iOS 11.3 caused react-beautiful-dnd to break: 
https://github.com/atlassian/react-beautiful-dnd/issues/413

It looks like this is also effecting shopify/draggable
https://github.com/Shopify/draggable/issues/198

[Radar WebKit Bug Importer]

<rdar://problem/39145092>

[Max Hoffmann]

I can confirm this for:

- Safari 11.3 on Mac OS X 10.13.3
- Safari 11.3 on iOS 11.3

Any drag and drop implementation for touch relies on calling `touchMoveEvent.preventDefault()` to cancel native scrolling. I could imagine most dnd solutions out there adding `touchmove` as child listeners.

The current workaround suggested by [e-mail] works well, but won't work when dynamically adding `touchmove` events to other documents, like iframes.

[Dean Jackson]

In the linked examples, I see the main problem being that we explicitly mark mousemove events as being not cancelable:

MouseEvent::create

bool isCancelable = eventType != eventNames().mousemoveEvent && !isMouseEnterOrLeave;

Whereas the spec says it should be:

https://www.w3.org/TR/uievents/#event-type-mousemove

However, it looks like we've implemented it that way since 2013, so this wouldn't be a recent regression.

For touch events, it's been similarly unchanged since 2014.

I think I'll have to make a different test case.

[Dean Jackson]

Yes, the touchmove is not cancelable either.

I was worried this might have something to do with making event listeners passive by default, but the examples here are explicitly setting to be not-passive.

[Dean Jackson]

I can confirm it is only happening to event listeners that are added inside event handlers. It also doesn't matter if the once option is true or false.

[Ernst]

The bug that dynamically added event listeners (i.e. event listeners added in event listeners) can't preventDefault exists since iOS 10: 
http://www.openradar.me/28359335
http://www.openradar.me/28479522

It's affecting my gestures library also.

I can confirm it has been working a long time until iOS 11.3!

[Tadeu Zagallo]

I think I finally understood how the behaviour actually changed. Dynamic touchmove listeners haven't been cancelable since iOS 10, so I couldn't find a minimal repro case at first, which was confusing, since the events *were* cancelable in the example provided here. What I realised was that the only reason the example worked in the first place was that there was already a touchmove listener on the document, added by React. So, before iOS 11.3, a touchmove event was cancelable as long as there was already one touchmove listener in the region of the touch. Since iOS 11.3, touchmove events cannot be prevented by dynamic listeners, regardless of whether there was already another listener. I have not yet identified what cause this regression though, but I'm working on that now that I have reduced the example.

[Tadeu Zagallo]

I can see now how making event listeners passive by default affected all the drag-and-drop libraries: Even though the event listener was explicitly setting passive to false, its behaviour depended on the event listener that React had added to the document first, as I mentioned in my previous comment. That event listener was being added to the document, and did not explicitly set passive to false, so it was being treated as a passive listener (handled asynchronously), which in turn caused the dynamically added listener to also be handled asynchronously. I could verify that by modifying the React's listener to pass passive: false and that restored the original behaviour of the drag-and-drop. I'll have a patch soon.

Is the patch to resume the pre 11.3 behaviour - or to fix the ability to enable calling event.preventDefault in a dynamically added event listener?

[Tadeu Zagallo]

(In reply to alexreardon from comment #12)
> Is the patch to resume the pre 11.3 behaviour - or to fix the ability to
> enable calling event.preventDefault in a dynamically added event listener?

It will make touch events cancelable from dynamically added event listeners.

Yay! 🎉

[Tadeu Zagallo]

Created attachment 340412 [details]
Patch

[Geoffrey Garen]

Comment on attachment 340412 [details]
Patch

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

r=me with comments

> LayoutTests/fast/events/touch/ios/touchmove-cancelable-after-touchstart.html:77
> +        // NOTE: removing this assertion will cause the test to fail, the event listener region is not immediately updated
> +        shouldBe("event.target", "square");

Let's clarify this comment:

(1) Let's say "For some reason, if we don't include this shouldBe line, the first few touchmove events we see will be passive (not cancelable)".

(2) Let's link to a bug report that says we should fix that.

(3) If all you need to work around that bug is a change to the DOM, then can we just do some kind of no-op DOM change? That will make the workaround clearer.

> LayoutTests/fast/events/touch/ios/touchmove-cancelable-after-touchstart.html:79
> +        window.addEventListener("touchmove", touchmoveEventHandler, { passive: false, once: true });
> +    }, { once: true });

I think it's a little racy for the web author to specify touchstart as passive. There's no guarantee that touchmove will be added before the user is done touching!

So, I think this test case would be a little better if we specified passive: false on the touchstart handler -- and it would still test our ability to treat the new touchmove handler as non-passive (synchronous).

[Tadeu Zagallo]

Created attachment 340433 [details]
Patch for landing

[Geoffrey Garen]

Comment on attachment 340433 [details]
Patch for landing

r=me

[WebKit Commit Bot]

Comment on attachment 340433 [details]
Patch for landing

Clearing flags on attachment: 340433

Committed r231821: <https://trac.webkit.org/changeset/231821>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Comment on attachment 340433 [details]
Patch for landing

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

> Source/WebKit/ChangeLog:9
> +        The tracking type for touch events were only update on touchstart, which meant that event

"were only update" -> "was only updated"

[Tadeu Zagallo]

Reopening to attach new patch.

[Tadeu Zagallo]

Created attachment 340528 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebKit/ChangeLog:168:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: LayoutTests/ChangeLog:146:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 2 in 2 files


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

[Geoffrey Garen]

Comment on attachment 340528 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 340528 [details]
Patch

Rejecting attachment 340528 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 340528, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog is not at the top of the file.

Full output: http://webkit-queues.webkit.org/results/7702520

[Geoffrey Garen]

Committed r231907: <https://trac.webkit.org/changeset/231907>

Thank you everyone for your efforts on this. 

Has the fix been released in Safari yet? How could I find this out?

I can see that it has not yet been released on Safari 11.1.2: https://codepen.io/alexreardon/pen/eMKMmm

It looks like it is now working for Safari on mobile: https://codepen.io/alexreardon/pen/yKEKby

[Ryosuke Niwa]

The fix should be included in the iOS 12 betas.

Great, thanks

Just tried the codepen https://codepen.io/alexreardon/pen/yKEKby on iOS 12.0 beta with Safari 12 on a real device, and the bug persists.

[Tadeu Zagallo]

There's another bug related to this one that hasn't been fixed yet, in which only the first few touches are not cancelable from dynamically added touch handlers: https://bugs.webkit.org/show_bug.cgi?id=185656. In this case, since both events are handled with `once: true`, this happens most of the time. The patch for that bug has not been landed yet.

[Andrew]

This is still broken on tests with iOS 12 public beta.