Closed Bug 1638458 Opened 4 years ago Closed 4 years ago

Long-pressing seems busted on Android

Categories

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

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox-esr68 --- unaffected
firefox76 --- unaffected
firefox77 --- unaffected
firefox78 --- fixed

People

(Reporter: kats, Assigned: botond)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Long-pressing on links should pop up a context menu with "open link in new tab" etc. However it seems to not do that any more, if you long-press on a link after zooming in. This is reproducible in Fenix and GVE.

STR:

  1. Go to https://staktrace.github.io/moz-pages/grid.html
  2. Zoom in
  3. Long-press on one of the links, e.g. (300, 400).

Expected:
In GVE the link should highlight red for a moment. I think GVE doesn't have an actual context menu implemented. In Fenix you should get a context menu.

Actual:
Doesn't work. Sometimes text selection is triggered instead.

I'm attempting bisection.

Bisected to bug 1556556

Blocks: 1630393
Regressed by: 1556556
Has Regression Range: --- → yes
Keywords: regression
Assignee: nobody → botond

Hmm. I thought the highlight / context menu would be triggered by the contextmenu event dispatched here, but that does not seem to be the case.

I was confusing the "context menu" with the "floating text selection toolbar" (which is technically also a kind of context menu).

The logic seems to be:

  • We dispatch a contextmenu event here.
  • Listeners (usually in the browser chrome, but maybe also in web content) can preventDefault() that event to indicate they intend to show a context menu.
  • If the event was not prevent-defaulted, we then dispatch an eMouseLongTap event here, which can trigger text selection via AccessibleCaret and the appearance of the floating text selection toolbar.

Listeners for the contextmenu event will choose to preventDefault() the event or not depending on what content was tapped (e.g. for GeckoView, there is some logic here which checks for links, images, and media).

So I think the observed behaviour that "sometimes text selection is triggered instead" is owing to the fact that sometimes we wrongly think we're not over a link even when we are.

Adding to the confusion is the fact that GVE will preventDefault() the contextmenu event if it thinks a link etc. was tapped, but not actually show a context menu.

One issue here is that when we dispatch the contextmenu event here, APZ is passing visual coordinates to APZCCallbackHelper::DispatchMouseEvent(), but that calls nsContentUtils::SendMouseEvent() which expects layout coordinates.

However, fixing that does not fix the problem, so there must be something else going wrong as well.

(In reply to Botond Ballo [:botond] from comment #4)

However, fixing that does not fix the problem, so there must be something else going wrong as well.

The other issue is that we are passing aIgnoreRootScrollFrame=true to DispatchMouseEvent(), which defeats the purpose of the layout/visual transformations we do.

The patch also documents APZCCallbackHelper::DispatchMouseEvent()
as expecting layout coordinates.

No one is setting this parameter to true any more.

Depends on D75734

The patch also removes the ignoreRootScrollFrame option from
APZCCallbackHelper::DispatchMouseEvent() altogether as it is
no longer used.

Depends on D75735

A test would be great too. It can be a follow up.

Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e9ee0a46ce7
Use layout coordinates when dispatching contextmenu event. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/661c530ea53a
Remove the ignoreRootScrollFrame parameter of FrameLoader.sendCrossProcessMouseEvent(). r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/8acda9da4ae7
Do not use ignoreRootScrollFrame when dispatching a contextmenu event in APZEventState. r=tnikkel
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: