Closed Bug 1492288 Opened Last year Closed Last year
[Android] Firefox's context menu doesn't appear (to offer e
.g . "paste"), when I long-press the password field at bugzilla .mozilla .org
7.52 MB, video/mp4
Bug 1492288 - Use IGNORE_ROOT_SCROLL_FRAME on Android when doing the hit test in AccessibleCaretManager::SelectWordOrShortcut(). r?kats
46 bytes, text/x-phabricator-request
|Details | Review|
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.
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.
Assignee: nobody → botond
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.)
The issue on this page is related to the main thread hit test being performed in AccessibleCaretManager::SelectWordOrShortcut() . When performing hit-testing for an event sent by APZ, elsewhere (e.g. in APZCCallbackHelper ) 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.  https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/layout/base/AccessibleCaretManager.cpp#563  https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/gfx/layers/apz/util/APZCCallbackHelper.cpp#683
Depends on D6550
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:firstname.lastname@example.org) has approved the revision.
Attachment #9011142 - Flags: review+
OS: Unspecified → Android
Priority: -- → P2
Version: Trunk → 63 Branch
Attachment #9011145 - Attachment is obsolete: true
Pushed by email@example.com: 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 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
Attachment #9011142 - Flags: approval-mozilla-beta?
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.
Attachment #9011142 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.