[webkit-reviews] review granted: [Bug 184250] Unable to call event.preventDefault in dynamically added event listener : [Attachment 340412] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Tadeu Zagallo
<tzagallo at apple.com>'s request for review:
Bug 184250: Unable to call event.preventDefault in dynamically added event
listener
https://bugs.webkit.org/show_bug.cgi?id=184250

Attachment 340412: Patch

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




--- Comment #16 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 340412
  --> https://bugs.webkit.org/attachment.cgi?id=340412
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).