1492288 - [Android] Firefox's context menu doesn't appear (to offer e.g. "paste"), when I long-press the password field at bugzilla.mozilla.org

Status: RESOLVED FIXED

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

Description

[Daniel Holbert [:dholbert]]

STR:
 0. Have some text in your clipboard.
 1. Visit https://bugzilla.mozilla.org/ in Firefox Nightly for Android
 2. Tap the "Log in" link at top right.
 3. Tap the password field to focus it.
 4. Long-press the password field.

ACTUAL RESULTS:
After a short pause (maybe a second), the phone vibrates and the password field loses focus. No menu appears (not onscreen at least).

EXPECTED RESULTS:
Context menu should appear, to offer "paste" as an option.

I ran mozregression and identified this range as the culprit:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b62d582090e11edcdd01c2c707e0b07b33ad0b12&tochange=ecafdbe899a7b2e574922250e356f8610788ad75

--> regression from bug 1423011.

This might possibly be a dupe of bug 1484597 (which is also marked as a regression from bug 1423011), but filing independently for the moment. Looks like that bug has a patch that may be landing soon, so I'll see if this is fixed after that lands.

Comment 1

[Botond Ballo [:botond]]

Hmmm. Testing this STR made me realize that copy/paste is broken even more badly on bugzilla.mozilla.org than on the website from bug 1484597 (in addition to the floating toolbar not appearing, the text selection handles don't appear either), and the patches in bug 1484597 dont't fix the brokenness on bugzilla.mozilla.org. Will investigate.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug

Here's a screencast of this bug reproducing, FWIW.

(You can't see me tapping/long-pressing, but you can see the effects -- after each long-press of the password field, the keyboard disappears and the password field loses focus.)

Comment 3

[Botond Ballo [:botond]]

The issue on this page is related to the main thread hit test being performed in AccessibleCaretManager::SelectWordOrShortcut() [1].

When performing hit-testing for an event sent by APZ, elsewhere (e.g. in APZCCallbackHelper [2]) we pass the IGNORE_ROOT_SCROLL_FRAME flag, and this is important for correct hit-testing in the presence of zooming (more precisely, in the presence of a divergence between the layout and visual viewport offsets, which happens more often when zoomed in with bug 1423011 in place).

Using IGNORE_ROOT_SCROLL_FRAME in the AccessibleCaretManager hit test as well fixes this issue.

[1] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/layout/base/AccessibleCaretManager.cpp#563
[2] https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/gfx/layers/apz/util/APZCCallbackHelper.cpp#683

Comment 4

[Botond Ballo [:botond]]

Attachment: Bug 1492288 - Use IGNORE_ROOT_SCROLL_FRAME on Android when doing the hit test in AccessibleCaretManager::SelectWordOrShortcut(). r?kats

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1492288 debugging

Depends on D6550

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 9011142 [details]
Bug 1492288 - Use IGNORE_ROOT_SCROLL_FRAME on Android when doing the hit test in AccessibleCaretManager::SelectWordOrShortcut(). r?kats

Kartikaya Gupta (email:[e-mail]) has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1645dd67b553
Use IGNORE_ROOT_SCROLL_FRAME on Android when doing the hit test in AccessibleCaretManager::SelectWordOrShortcut(). r=kats

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/1645dd67b553

Comment 9

[Botond Ballo [:botond]]

Comment on attachment 9011142 [details]
Bug 1492288 - Use IGNORE_ROOT_SCROLL_FRAME on Android when doing the hit test in AccessibleCaretManager::SelectWordOrShortcut(). r?kats

Approval Request Comment
[Feature/Bug causing the regression]:
  Bug 1423011

[User impact if declined]:
  When zoomed in and doing a long-press, the floating
  toolbar that comes up may be positioned incorrectly,
  including potentially being completely offscreen.

[Is this code covered by automated tests?]:
  The text selection code has some test coverage,
  but this specific scenario does not have a test.

[Has the fix been verified in Nightly?]:
  Yes, by me.

[Needs manual test from QE? If yes, steps to reproduce]: 
  No.

[List of other uplifts needed for the feature/fix]:
  Bug 1484597

[Is the change risky?]:
  Moderately risky

[Why is the change risky/not risky?]:
  This is a very small change, but it's difficult to rule
  out unintended consequences. (Note that any hypothetical
  unintended consequences would be limited to text selection 
  scenarios.)

[String changes made/needed]:
  None

Comment 10

[Pascal Chevrel:pascalc]

Comment on attachment 9011142 [details]
Bug 1492288 - Use IGNORE_ROOT_SCROLL_FRAME on Android when doing the hit test in AccessibleCaretManager::SelectWordOrShortcut(). r?kats

P2 regression on 63, on nightly for 4 days without new regressions reported, uplift accepted for 63 beta 11, thanks.

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c1f74cb53e6d