Long-pressing seems busted on Android
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
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:
- Go to https://staktrace.github.io/moz-pages/grid.html
- Zoom in
- 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.
Reporter | ||
Comment 1•4 years ago
|
||
Bisected to bug 1556556
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
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.
Assignee | ||
Comment 3•4 years ago
|
||
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.
Assignee | ||
Comment 4•4 years ago
|
||
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.
Assignee | ||
Comment 5•4 years ago
|
||
(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.
Assignee | ||
Comment 6•4 years ago
|
||
The patch also documents APZCCallbackHelper::DispatchMouseEvent()
as expecting layout coordinates.
Assignee | ||
Comment 7•4 years ago
|
||
No one is setting this parameter to true any more.
Depends on D75734
Assignee | ||
Comment 8•4 years ago
|
||
The patch also removes the ignoreRootScrollFrame option from
APZCCallbackHelper::DispatchMouseEvent() altogether as it is
no longer used.
Depends on D75735
Comment 9•4 years ago
|
||
A test would be great too. It can be a follow up.
Comment 10•4 years ago
|
||
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
Comment 11•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1e9ee0a46ce7
https://hg.mozilla.org/mozilla-central/rev/661c530ea53a
https://hg.mozilla.org/mozilla-central/rev/8acda9da4ae7
Description
•