908861 - Prevent dismissal of text selection handles during drag

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Text Selection, defect)

Description

[Mark Capella [:capella]]

Attachment: bugTextSelection

"TextSelection:Move" and "TextSelection:Position" logic causes on the fly / transient selectionChanged messages ... causing notifySelectionChanged() to believe selected text no longer exists.

1) Visit a text based page
2) Long Tap text to make highlight selection / display handles
3) Drag left handle to right past right handle
4) Handles dissappear


This patch corrects by ignoring selectionChanged messages while handles are moving / selection is in a transient state.

I considered dynamically removing and re-adding the selectionListener() in the appropriate places ... but lean towards this solution as tighter / fast (?)

Comment 1

[:Margaret Leibovic]

So is this a regression from bug 864589? We should make sure to fix this before shipping 26 (we still have plenty of time, but we should track this).

Comment 2

[:Margaret Leibovic]

Comment on attachment 794840 [details] [diff] [review]
bugTextSelection

Review of attachment 794840 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, I tested with this and can confirm it fixes the problem.

::: mobile/android/chrome/content/SelectionHandler.js
@@ +15,5 @@
>    // Keeps track of data about the dimensions of the selection. Coordinates
>    // stored here are relative to the _contentWindow window.
>    _cache: null,
>    _activeType: 0, // TYPE_NONE
> +  _isSelectionTransient: false, // True while user drags text selection handles

I would prefer to name this something like _ignoreSelectionChanges, since that makes it clearer what this flag is all about.

Comment 3

[Mark Capella [:capella]]

I know I've worked on several of these lately, so I had to check. This patch fell out of bug 903316 ...

_ignoreSelectionChanges is good ... I wrestled over something appropriate and just settled on _isSelectionTransient for lack of a better idea.

Comment 4

[Mark Capella [:capella]]

Attachment: bug908861

So this would be the final ...

Comment 5

[:Margaret Leibovic]

Comment on attachment 796383 [details] [diff] [review]
bug908861

Review of attachment 796383 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.

Comment 6

[Mark Capella [:capella]]

Push to TRY
https://tbpl.mozilla.org/?tree=Try&rev=58f1986302fa

Comment 7

[Mark Capella [:capella]]

That went well, so next we go:
https://hg.mozilla.org/integration/fx-team/rev/b69456879f40

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b69456879f40