Closed
Bug 1338445
Opened 7 years ago
Closed 7 years ago
Long tapping in block element which has only a short word selects odd range on Android
Categories
(Core :: DOM: Selection, defect)
Tracking
()
VERIFIED
FIXED
mozilla54
People
(Reporter: masayuki, Assigned: TYLin)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file)
STR1: 1. load https://jsfiddle.net/d_toybox/d5y5sa4x/8/ on Android 2. long tap at right blank space of "p", "p2", "c" or "c2" in the table. Expected result: Only "p", "p2", "c" or "c2" is selected (selection range is in a text node). Actual result: "c" and "c2" cases are fine but "p" and "p2" cases odd. Selection is started from the last of the left cell and ended at the start of the right cell. STR2: 1. Load https://jsfiddle.net/d_toybox/d5y5sa4x/8/ 2. Long tap at right space of "p" below the table. Expected Result: Only "p" is selected in the <p> element. Actual Result: Selected from the start of the previous <p> element of <div> to the start of the next text node under <body>. If the text is long word, I cannot reproduce this bug. I don't know how to reproduce this on desktop...
Reporter | ||
Comment 1•7 years ago
|
||
I reproduced this bug both with release (51.0.2) and Nightly.
Reporter | ||
Updated•7 years ago
|
Assignee | ||
Comment 2•7 years ago
|
||
To reproduce this on desktop, one can flip the following prefs to mimic Fennec. 1) layout.accessiblecaret.enabled -> true 2) layout.accessiblecaret.extend_selection_for_phone_number -> true 3) layout.accessiblecaret.hide_carets_for_mouse_input -> false 4) layout.accessiblecaret.use_long_tap_injector -> true This must be the phone number selection extension enabled only on Fennec that causes the odd range. AccessibleCaretManager::ExtendPhoneNumberSelection() will extend the selected word both forward and backward until it cannot match the regex [1]. The test cases "p" and "p2" happen to match the regex. BTW, the feature was ported from js to C++ in bug 1235508, and the original phone number regex is here [2]. [1] http://searchfox.org/mozilla-central/rev/ea31c4b64f34a29415a69fb768f8051495547315/layout/base/AccessibleCaretManager.cpp#973 [2] http://searchfox.org/mozilla-central/rev/4428147ca5f614cf3a33ffce1f8b60acc4e6a458/mobile/android/chrome/content/SelectionHandler.js#1152
Assignee | ||
Comment 3•7 years ago
|
||
I feel "\\s" (matching a white space char) in the regex "(^\\+)?[0-9\\s,\\-.()*#pw]{1,30}$" might match too much. It'll match something like "\n" or "\t", which should not appear in a valid phone number string. A plain space char ' ' should be sufficient.
Comment hidden (mozreview-request) |
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8838820 [details] Bug 1338445 - Restrict whitespace match in phone number regex for AccessibleCaret. https://reviewboard.mozilla.org/r/113630/#review115318
Attachment #8838820 -
Flags: review?(mtseng) → review+
Pushed by tlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/da077f0a94ce Restrict whitespace match in phone number regex for AccessibleCaret. r=mtseng
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/da077f0a94ce
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Reporter | ||
Comment 8•7 years ago
|
||
Thank you for your work. I confirmed that this is fixed on the Nightly. However, I still see similar problem on a website which I found the original problem. I'll file another bug after I create a testcase for that. Then, I'll cc you to new bug.
Status: RESOLVED → VERIFIED
Comment 9•7 years ago
|
||
Too late for 52. Would it make sense to uplift this to beta53?
Flags: needinfo?(tlin)
Assignee | ||
Comment 10•7 years ago
|
||
This is a long-standing bug, and the fix is risky. I'll just let it ride the train.
Flags: needinfo?(tlin)
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•