Closed
Bug 1224884
Opened 9 years ago
Closed 9 years ago
GeckoCarets (either Touch/Selection or Accessible) LongTap to SelectWord while composing corrupts editable data
Categories
(Firefox for Android Graveyard :: Text Selection, defect)
Firefox for Android Graveyard
Text Selection
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1082425
People
(Reporter: capella, Assigned: capella)
References
Details
User Story
See example video: [0]. This was not an issue when Android w/Gecko Carets first went in bug 988143, so I'm assuming something changed. This started annoying me during testing bug 1215959, Touch/SelectionCarets ---> AccessibleCarets, but appears to pre-date to the current version. Not sure where this regressed or why or if it matters. The fastest thing might just be to call composition clear at start of WordSelection AccessibleCarets after [1], though there may be a more appropriate place back up the LongPress chain. [0] https://www.dropbox.com/s/8jv9nsw2tmk4n2u/bug_GeckoCarets2.mp4?dl=0 [1] http://mxr.mozilla.org/mozilla-central/source/layout/base/AccessibleCaretManager.cpp?rev=1650689e3fdb&mark=384-384#382
Attachments
(1 file)
1.99 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•9 years ago
|
Blocks: GeckoCaret2
User Story: (updated)
Assignee | ||
Comment 1•9 years ago
|
||
This works for me. Jchen, is there something different you can suggest here? This shouldn't be a problem, but the solution seems inelegant.
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #8690118 -
Flags: review?(tlin)
Attachment #8690118 -
Flags: review?(nchen)
Comment 2•9 years ago
|
||
Comment on attachment 8690118 [details] [diff] [review] bug1224844.diff Review of attachment 8690118 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I'm not familiar with this code. I do remember you had to add something to GeckoEditable [1] for it to play nicely with the old text selection code. Not sure if this is along the same vein as that bug. [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoEditable.java?rev=0b2b0570777f#1370
Attachment #8690118 -
Flags: review?(nchen)
Comment 3•9 years ago
|
||
Comment on attachment 8690118 [details] [diff] [review] bug1224844.diff Review of attachment 8690118 [details] [diff] [review]: ----------------------------------------------------------------- Mark, This bug is similar to bug 1082425. Since B2G has this issue as well, I would like to investigate this myself, and I hope I can come up with a nicer solution for all platform. ::: layout/base/AccessibleCaretManager.cpp @@ +662,5 @@ > + // Explicitly clear before re-focus, when LongTapping to start a new selection > + // in the currently focused field & in Cursor mode, to reset Fennec IME in > + // cases where on-the-fly autoSuggestions are present. > + if ((fm->GetFocusedContent() && fm->GetFocusedContent() == focusableContent) && > + (GetCaretMode() == CaretMode::Cursor && sCaretsExtendedVisibility)) { sCaretsExtendedVisibility is not define yet.
Attachment #8690118 -
Flags: review?(tlin)
Assignee | ||
Comment 4•9 years ago
|
||
Ah, ok, sure :) We have a couple weeks before this gets exposed by bug 1215959 (which is where I add the pref |sCaretsExtendedVisibility| ... Cool! I see in bug 1082425 there's talk of using REQUEST_TO_COMMIT_COMPOSITION, which is something I was wondering here also. tylin, I'll wait and see what you come up with there :-)
Assignee | ||
Comment 5•9 years ago
|
||
tylins got the right way ... closing dup over to him
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•