GeckoCarets (either Touch/Selection or Accessible) LongTap to SelectWord while composing corrupts editable data

RESOLVED DUPLICATE of bug 1082425

Status

()

Firefox for Android
Text Selection
RESOLVED DUPLICATE of bug 1082425
3 years ago
2 years ago

People

(Reporter: capella, Assigned: capella)

Tracking

Firefox Tracking Flags

(Not tracked)

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 attachment)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Blocks: 1215959
User Story: (updated)
(Assignee)

Comment 1

2 years ago
Created attachment 8690118 [details] [diff] [review]
bug1224844.diff

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 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 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

2 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

2 years ago
tylins got the right way ... closing dup over to him
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1082425
You need to log in before you can comment on or make changes to this bug.