Prevent dismissal of text selection handles during drag

RESOLVED FIXED in Firefox 26



Firefox for Android
Text Selection
5 years ago
4 years ago


(Reporter: capella, Assigned: capella)


Firefox 26

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



5 years ago
Created attachment 794840 [details] [diff] [review]

"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 (?)
Attachment #794840 - Flags: feedback?(margaret.leibovic)

Comment 1

5 years ago
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).
tracking-fennec: --- → ?

Comment 2

5 years ago
Comment on attachment 794840 [details] [diff] [review]

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.
Attachment #794840 - Flags: feedback?(margaret.leibovic) → feedback+

Comment 3

5 years ago
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

5 years ago
Created attachment 796383 [details] [diff] [review]

So this would be the final ...
Attachment #794840 - Attachment is obsolete: true
Attachment #796383 - Flags: review?(margaret.leibovic)

Comment 5

5 years ago
Comment on attachment 796383 [details] [diff] [review]

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

Looks good, thanks.
Attachment #796383 - Flags: review?(margaret.leibovic) → review+

Comment 7

5 years ago
That went well, so next we go:
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.