Closed Bug 865356 Opened 7 years ago Closed 7 years ago

Defect - Difficult to select the text you intend to select in text input fields

Categories

(Firefox for Metro Graveyard :: Input, defect, P2)

x86_64
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 24

People

(Reporter: jbecerra, Assigned: jimm)

References

Details

(Whiteboard: [selection][shovel-ready]feature=defect c=Content_features u=metro_firefox_user p=2)

Attachments

(2 files)

Tested on 2013-04-24 using a nightly built from http://hg.mozilla.org/mozilla-central/rev/fef5f202b2dc

While verifying bug 862025 I noticed that I had a very hard time selecting the word I wanted to select in the text field.

Steps:
1. Go to https://bug862025.bugzilla.mozilla.org/attachment.cgi?id=737631
- When the page loads you will see some text pre-filled in the field.
2. Tap on the word "tunnel" and try to select that word.
- The virtual keyboard comes up and the monocle appears
3. Try dragging the monocle to select the word.

Expected: You can drag the monocle to select the word or words, and those remain in your field of view.

Actual: The monocle seems to disappear and the caret starts to blink at the end or the beginning of the text in the field. From there you have to tap again to see the monocles and fish for the word by dragging left or right until it appears.
Whiteboard: feature=defect c=tbd u=tbd p=tbd
Blocks: 862025
Whiteboard: feature=defect c=tbd u=tbd p=tbd → feature=defect c=Content_features u=metro_firefox_user p=tbd
Component: General → Input
OS: Windows 8 → Windows 8 Metro
Summary: defect - it's difficult to select the text you intend to select in text input fields → Defect - Difficult to select the text you intend to select in text input fields
Whiteboard: feature=defect c=Content_features u=metro_firefox_user p=tbd → feature=defect c=Content_features u=metro_firefox_user p=0
Depends on: 865660
Whiteboard: feature=defect c=Content_features u=metro_firefox_user p=0 → [selection] feature=defect c=Content_features u=metro_firefox_user p=0
Priority: -- → P2
Blocks: 867657
:jimm, does this need anything for someone to start digging? Any ideas what is going on under the hood?
(In reply to :Ally Naaktgeboren from comment #1)
> :jimm, does this need anything for someone to start digging? Any ideas what
> is going on under the hood?

This is good to go. Just a guess, but I think this might have something do do with this call - 

http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/contenthandlers/SelectionHandler.js#202

I think the focus/anchor nodes are getting mixed up for some reason after an initial drag in the same edit. Shouldn't be too hard to track down.

The fix should also include a selection test.
Whiteboard: [selection] feature=defect c=Content_features u=metro_firefox_user p=0 → [selection][shovel-ready]feature=defect c=Content_features u=metro_firefox_user p=0
Assignee: nobody → jmathies
Whiteboard: [selection][shovel-ready]feature=defect c=Content_features u=metro_firefox_user p=0 → [selection][shovel-ready]feature=defect c=Content_features u=metro_firefox_user p=2
Attached file test case
Blocks: metrov1it8
No longer blocks: metrov1defect&change
QA Contact: jbecerra
This is the result of a bad assumption in the caret positioning code. Normally caret moves are centered on tap coordinates in text inputs, but we also send one right at the start of a drag move in which case the coord is actually centered on the monocle, just below the edit. In this case, caretPositionFromPoint returns an random offset which totally hoses up caret focus.

The other drag events already compensate for this.

Note this is still a little busted for text inputs with large css heights due to bug 865654. :jwir3's been working on that so hopefully it'll be fixed soon.
Attachment #753410 - Flags: review?(mbrubeck)
Status: NEW → ASSIGNED
Comment on attachment 753410 [details] [diff] [review]
fix - restrict caret move coords to edit bounds

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

::: browser/metro/base/content/contenthandlers/SelectionHandler.js
@@ +620,5 @@
> +    let bounds = this._getTargetBrowserRect();
> +    if (aX < bounds.left)
> +      result.xPos = bounds.left + 1;
> +    if (aX > bounds.right)
> +      result.xPos = bounds.right - 1;

Should these be <= and >=, so the result will always be in [left + 1, right -1]?
Attachment #753410 - Flags: review?(mbrubeck) → review+
(In reply to Matt Brubeck (:mbrubeck) from comment #5)
> Comment on attachment 753410 [details] [diff] [review]
> fix - restrict caret move coords to edit bounds
> 
> Review of attachment 753410 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/metro/base/content/contenthandlers/SelectionHandler.js
> @@ +620,5 @@
> > +    let bounds = this._getTargetBrowserRect();
> > +    if (aX < bounds.left)
> > +      result.xPos = bounds.left + 1;
> > +    if (aX > bounds.right)
> > +      result.xPos = bounds.right - 1;
> 
> Should these be <= and >=, so the result will always be in [left + 1, right
> -1]?

Yeah that sounds good to me.
https://hg.mozilla.org/mozilla-central/rev/c86aa289455a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
User Agent : Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20130701 Firefox/25.0
WFM
I tested on random Bugzilla page for text selection in input fields using FX 25 nightly build.
Tested this for iteration 9.
WFM for latest nightly from ftp://ftp.mozilla.org/pub/firefox/nightly/2013-07-01-mozilla-central-debug
Status: RESOLVED → VERIFIED
Blocks: caret-sel
User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130807030216
Built from http://hg.mozilla.org/mozilla-central/rev/1fb5d14e8348

WFM
Tested on windows 8 using latest nightly for iteration-11. Followed steps provided in comment0 and got expected result.
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130819030205
Built from http://hg.mozilla.org/mozilla-central/rev/c8c9bd74cc40

WFM
Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment0 and got expected result.
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.