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)

defect
Not set
normal

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)

      No description provided.
Blocks: GeckoCaret2
User Story: (updated)
Attached patch bug1224844.diffSplinter Review
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)
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   :-)
tylins got the right way ... closing dup over to him
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: