1481923 - [Android] preventDefault() in contextmenu listener cancels touch event generation (sends touchcancel)

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[CoolCmd]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180704003137

Steps to reproduce:

see the test case: https://jsfiddle.net/CoolCmd/4b8faq3v/

1. touch and hold finger on 1st (from top) rectangle.
2. close the "select all" context menu.
3. touch and hold finger on 2nd rectangle.



Actual results:

1. context menu "select all" was shown.
3. "touchcancel" event was shown in log (bottom rectangle).



Expected results:

1. browser's context menu should not appear.
3. preventDefault() in "contextmenu" event handler should not cancel touch events.

this bug prevents the use of modern API, such as pointer events and touch-action. firefox for android does not support pointer events yet (LOL), but this does not matter.

with this bug, sites that do not know about firefox for android will fail to work in some situations. sites that know about this bug should use non-passive events, which blocks rendering.

chrome for android works as expected. i do not test other browsers.

******* BONUS ********

other strange behavior:

 - you can not close the "select all" context menu from step 2 by tapping the white background (<IFRAME>).

- while "select all" is shown, touch and hold the finger on 1st rectangle. text selection will start. after that, this gesture always starts selection, although this is forbidden by touch-action: none.

Comment 1

[CoolCmd]

BONUS #2

"select all" menu is shown even after switching to new tab :)

Comment 2

[Edgar Chen [:edgar]]

I was trying to reproduce in Nightly 63 (2018-08-20), but I didn't see context menu was shown for 2nd rectangle.

Here is my step to reproduce,
1. touch and hold finger on 1st rectangle. (the context menu is shown)
2. close context menu by selecting "copy". (not sure if this is the same way to close the "select all" context menu mentioned in comment #0).
3. touch and hold finger on 2nd rectangle.

The result:
1. I didn't see the context menu was shown.
2. I saw "touchcancel" event shown in the log.

And the Chrome shows the "touchcancel" event as well.

Comment 3

[Edgar Chen [:edgar]]

BTW, I tested 61, and I didn't see context menu shown for 2nd rectangle, either.

Comment 4

[CoolCmd]

:edgar, 1st step in 'Actual results' refers to the 1st step in 'Steps to reproduce'. 3rd to 3rd. The same with 'Expected results'.

OK, forget about comment #0, here a simplified bug description:

I want to do some action (auto increment the number, drag & drop, etc) on "touch and hold" gesture. To do this i must disable browser's context menu. "touch-action" does not disable the menu in Firefox and Chrome. The second option: call event.preventDefault() in "contextmenu" handler. This works perfectly in Chrome, but useless in Firefox, because it cancels touch events generation: browser immediately sends the 'touchend' event! Instead in Firefox i must cancel the 'touchstart' and 'touchend' events, which is bad practice. Firefox behavior is incompatible with Chrome and likely with Safari.

I updated the test case here: https://jsfiddle.net/CoolCmd/4b8faq3v/

Comment 5

[Edgar Chen [:edgar]]

Thanks for the clarification.

And here is a minimal test script shows the difference: https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=6153
- In Gecko, we send `touchcancel` right after `contextmenu`.
- In Chrome, it doesn't send 'touchcancel', but send `touchend` after touch released.

Comment 6

[Edgar Chen [:edgar]]

Per https://www.w3.org/TR/touch-events/#the-touchcancel-event, it looks like calling preventDefault() in contextmenu is not the case to fire touchcancel and we probably should fix this. Any thought, smaug?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

(better to not use /TR/ specs. They are very often obsolete.)
https://w3c.github.io/touch-events/#event-touchcancel

yeah, I don't know why we send touchcancel. touchend feels more reasonable.

Comment 8

[Edgar Chen [:edgar]]

Er.

Comment 9

[Sebastian Glatz]

Hi Guys :)

This behavior has turned out to be a serious problem while implementing logic which is based on touch and hold interaction for a web based framework I'm working on. Calling preventDefault from the touchstart event handler causes further problems, as other defaults are prevented too in addition to the context menu approach. And its working fine with all major Desktop Browser (including Firefox) and the most (haven't tested with a lot of smaller mobile browser packages so far) important mobile browsers.

So i decided to try my luck and bump that issue item up again :)

I would appreciate if someone of you could give an estimate if this issue will be fixed in foreseeable future or if i have to live with this issue for longer.

Comment 10

[Edgar Chen [:edgar]]

The touchcancel is from https://searchfox.org/mozilla-central/rev/0dfbe5a699cc6c73cf8c14d1aa10ba10ef3ec8fa/gfx/layers/apz/util/APZEventState.cpp#282-291.

Comment 11

[Edgar Chen [:edgar]]

Attachment: Bug 1481923 - Preventdefaulting a contextmenu event don't cause a touchcancel on Android;

The original behavoir was added in Bug 1231570 where we found a webpage did some
tricky things on touchend that causes navigation while we long-tap on a link.

However, other browsers doesn't send touchcancel event anymore when we prevent-default
a contextmenu event, so remove this as well for web compatibility. If we encounter
the similar issue as Bug 1231570 again, we probably need to check how does the
page behave on other browsers, and does the page do different things via UA Sniffing.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

CCing Karl since I believe this is a sort of webcompat issue. I wonder whether he has ever seen this kind of issue.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

kats left a very important comment in that bug from bug 1231570 comment 14;

After long-pressing on the link, the context menu popup makes Android send us a touchup, we should figure out a way to detect that and turn it into a touchcancel.

And there is another interesting comment we should care (from bug bug 1231570 comment 15);

Yeah, pre-APZ fennec and Chrome both send touchcancel after a handled contextmenu event, so we should be doing the same, probably in C++ code somewhere. I'm just not sure where the right place to do it is. Randall, any thoughts?

So, I believe the original case in bug 1231570 still applicable, but what we should really do is to fire a touchcancel even only if context menu opens. That means we need to differentiate the preventDefault() call in GeckoView when opening context menu. Luckily in the GeckoView's preventDefault() call, mDefaultPreventedByChrome flag is true, so that we can tell the difference.

Comment 15

[Karl Dubost💡 :karlcow]

So from the top of my head, I don't remember webcompat issues about this but I agree that this should be fixed.

And the differences create headaches for devs. Now the risk is that if Mozilla fixes it, will it break sites. The first step would be to try in Nightly and sees if there are any webcompat issues reported.

There is also an open issue about it.
Define interactions with contextmenu event?

Comment 16

[Karl Dubost💡 :karlcow]

Using https://codepen.io/webcompat/pen/gOmRKee
On Mobile

• Firefox Nightly 90.0a1 Build #2015812491
• Chrome Canary 92.0.4506.0
• Safari 14.1

Firefox

• touchstart
• contextmenu (no vibration)
• touchcancel

Chrome

• touchstart
• contextmenu (vibration)
• touchend

Safari

• touchstart
• (text selection is showed then removed + vibration. no contextmenu event)
• touchend

Comment 17

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1481923 - Make nsContentUtils::SendMouseEvent return the information where preventDefault() was called. r?masayuki

On a long press touch event we fire a contextmenu event and if the contextmenu
is opening, we fire a touchcancel event. Unfortunately we had been checking
the return value, nsEventStatus, from nsIWidget::DispatchInputEvent via
nsContentUtils::SendMouseEvent to tell whether the context menu is opening or
not. So, we unintentionally fire the touchcancel event if the context menu is
NOT going to be opened because of preventDefault() in a contextmenu event
listener in the content itself.

NOTE: The oparator<< for the new PreventDefaultResult enum will be used in
APZ logging code.

[1] https://searchfox.org/mozilla-central/rev/95c41d54c3fd65d51976d5188842a69b459a7589/mobile/android/actors/ContentDelegateChild.jsm#100

Comment 18

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1481923 - Fire a touchcancel event on contextmenu events only if our own context menu is opened on Android. r?botond

This approach is applicable for Android only since Android is the only one
platform we call preventDefault() [1] when opening context menu.

[1] https://searchfox.org/mozilla-central/rev/95c41d54c3fd65d51976d5188842a69b459a7589/mobile/android/actors/ContentDelegateChild.jsm#100

Depends on D115963

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Though I've finished writing a reasonable fix, firing a touchcancel event only if context menu is opened. That said, I am not yet convinced whether we should fire the touchcancel event when the web site opens its own context menu such as Google Docs or something like that. We also need to check what Chrome does on Google Docs at least. Note that with the fix I posted we probably no longer fire the touchcancel when opening the Google Doc's context menu (I haven't checked though).

(In reply to Karl Dubost💡 :karlcow from comment #15)

So from the top of my head, I don't remember webcompat issues about this but I agree that this should be fixed.

And the differences create headaches for devs. Now the risk is that if Mozilla fixes it, will it break sites. The first step would be to try in Nightly and sees if there are any webcompat issues reported.

Yeah, agree. we should try it in Nightly first.

Comment 20

[Hiroyuki Ikezoe (:hiro)]

I just checked a Google document on Fenix, and it looks like they don't allow editing documents on browsers (I've also checked on mobile Chrome), so we can't see their own context menu. And Karl also tried to see it on Microsoft Office sites, he said it's the same situation as what Google Docs does.
So I suppose presumably it doesn't much matter even if we no longer fire touchcancel events when preventDefailt() was called in contextmenu eventhandlers in contents, since it will also mach Chrome and Safari behaviors as Karl described in comment 16.

Comment 21

[Edgar Chen [:edgar]]

Thanks for looking into this, :hiro. I think your solution is more reasonable than mine :)
Since you already have a patch, hand this bug to you.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2cdf4ddd5141
Make nsContentUtils::SendMouseEvent return the information where preventDefault() was called. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/83495e0289f9
Fire a touchcancel event on contextmenu events only if our own context menu is opened on Android. r=botond

Comment 23

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2cdf4ddd5141
https://hg.mozilla.org/mozilla-central/rev/83495e0289f9