Last Comment Bug 675920 - Difficult to select text after these steps
: Difficult to select text after these steps
Status: VERIFIED FIXED
:
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: Firefox 7
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
Depends on:
Blocks: 670222
  Show dependency treegraph
 
Reported: 2011-08-02 06:43 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2011-08-04 20:17 PDT (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.79 KB, patch)
2011-08-02 14:28 PDT, Mark Finkle (:mfinkle) (use needinfo?)
wjohnston2000: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-02 06:43:14 PDT
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 [:mwargers] (not working for Mozilla) 2011-08-02 06:47:51 PDT
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 [:mwargers] (not working for Mozilla) 2011-08-02 08:12:49 PDT
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 [:mwargers] (not working for Mozilla) 2011-08-02 08:25:38 PDT
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 [:mwargers] (not working for Mozilla) 2011-08-02 08:29:28 PDT
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?) 2011-08-02 14:28:12 PDT
Created attachment 550211 [details] [diff] [review]
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) 2011-08-02 15:03:29 PDT
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?) 2011-08-02 21:37:28 PDT
http://hg.mozilla.org/mozilla-central/rev/3735fb1cd5ef

Please re-verify bug 674515 as well.
Comment 8 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-08-03 05:34:15 PDT
Verified fixed in today's Nightly on the LG Optimus Black. Thanks for fixing!
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-04 12:37:11 PDT
Comment on attachment 550211 [details] [diff] [review]
patch

mobile only and it helps stabilize the text selection feature
Comment 10 christian 2011-08-04 14:48:04 PDT
Comment on attachment 550211 [details] [diff] [review]
patch

Approved for releases/mozilla-aurora. Please land ASAP.
Comment 11 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-04 20:17:33 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/029a0c4af935

Note You need to log in before you can comment on or make changes to this bug.