675920 - Difficult to select text after these steps

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Steps to reproduce:
- Go to a site with some text in it
- Tap down and hold on some text, until the text selection and the handles come up
- Now Pan down at some other spot than the selection, while holding your finger on the screen, then release.
- Tap down and hold on some text again.

Expected result:
- The text should become selected with the selection handles visible.

Actual result:
- No text select, no selection handles.

Comment 1

[Martijn Wargers (dead)]

Ah wait, never mind, correct steps to reproduce:
1. Go to a site with some text in it
2. Tap down and hold on some text, until the text selection and the handles come up
3. Pan down at some other spot than the selection, then tap on the screen to stop the panning down.
4. Tap down and hold on some text again.

Notice that in step 3, you get this error in the error console:
this._dragger is null
chrome://browser/content/input.js
409

(I thought I filed a bug about this already, but I can't seem to find it)

I guess that error might be causing this issue.

Comment 2

[Martijn Wargers (dead)]

It turns out to be more tricky to reproduce this, than I thought.
For some reason it's easier to reproduce on bugzilla pages like this one, for starters.

Comment 3

[Martijn Wargers (dead)]

I posted a video here. It's not really with the steps to reproduce, but at least it shows you that I'm not crazy.

Comment 4

[Martijn Wargers (dead)]

Sorry, forgot the link to the video: http://www.youtube.com/watch?v=naagyVipAEw

Btw, I filed bug 675950 for the js error mentioned in comment 1.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch reverts the fix for bug 674515 and uses a different approach. The old fix for bug 674515 forced the finger tap to and up in the selected word, making it possible for tapping on whitespace near a word to cause the selection to not happen.

This patch first clears any existing ranges, then attempts to select the word near the tap. Tapping on whitespace will still allow the selection to happen.

The patch still fixes bug 674515. I also added a null check for "elem".

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 550211 [details] [diff] [review]
patch

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

::: mobile/chrome/content/content.js
@@ +1367,5 @@
>            utils = elem.contentDocument.defaultView.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
>            elem = utils.elementFromPoint(x, y, true, false);
>          }
> +        if (!elem)
> +          return;

I'm surprised we need to do this at all.... ever. I guess this means the coordinates are outside this document.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/mozilla-central/rev/3735fb1cd5ef

Please re-verify bug 674515 as well.

Comment 8

[Martijn Wargers (dead)]

Verified fixed in today's Nightly on the LG Optimus Black. Thanks for fixing!

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 550211 [details] [diff] [review]
patch

mobile only and it helps stabilize the text selection feature

Comment 10

[LegNeato]

Comment on attachment 550211 [details] [diff] [review]
patch

Approved for releases/mozilla-aurora. Please land ASAP.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/029a0c4af935