Closed Bug 1174471 Opened 5 years ago Closed 5 years ago

Leftarrow/rightarrow (and ctrl+rightarrow) jumps to the end of URL, if Unified Autocomplete dropdown has just appeared

Categories

(Firefox :: Address Bar, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 41
Tracking Status
firefox41 + verified

People

(Reporter: dholbert, Assigned: mossop)

References

Details

(Keywords: dogfood, regression, Whiteboard: [unifiedautocomplete][fxsearch])

Attachments

(3 files, 1 obsolete file)

STR:
 1. Visit e.g. https://wiki.mozilla.org/Main_Page
 2. Ctrl+L to focus URL bar
 3. Hold Ctrl and press rightarrow/leftarrow a few times; notice how cursor jumps to next punctuation mark.
 4. Now, with the cursor towards the beginning (say, just before the "w" of "wiki", type a letter, e.g. 'a'.
 5. Press Ctrl+rightarrow again.

EXPECTED RESULTS: Cursor should jump to next punctuation mark, as it did in step 3.

ACTUAL RESULTS: Cursor jumps to the *very end of the URL*.


I've run up against this several times in the past few days, but I only just figured out concrete STR. I think this is a recent regression in Nightly.


Using Nightly 41.0a1 (2015-06-12)
I can't reproduce in a Nightly from a month ago: 41.0a1 (2015-05-18)
In newer builds that reproduce this, in step 4 of STR, a dropdown appears that says e.g.
  "Visit https://awiki.mozilla.org/Main_Page"

I suspect this dropdown is what's breaking Ctrl+backarrow/forwardarrow.
Regression range on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bea6758824d9&tochange=6b20dd2e20cc

...which includes:
6968dac17cdf	Marco Bonardo — Bug 1168811 - Enable Unified AutoComplete. r=Mossop

...which just flipped browser.urlbar.unifiedcomplete to 'true'.

I verified that this bug (and the dropdown described in comment 2) goes away, if I disable that pref. So, looks like this is a bug with Unified Autocomplete.

This bug makes it very annoying to edit long URLs in-place in the URL bar, btw. I'm used to Ctrl+rightarrow being a quick way to jump around, and it's jarring to have it occasionally take me all the way to the end.
Flags: needinfo?(mak77)
Summary: Ctrl+rightarrow now jumps to the end of URL, if you've just typed a character → Ctrl+rightarrow now jumps to the end of URL, if you've just typed a character. (with Unified Autocomplete enabled)
Flags: firefox-backlog+
Whiteboard: [unifiedautocomplete][fxsearch]
I suspect rather than being a bug in unified complete, this is due to completedefaultindex that unified complete uses "correctly" while the previous autocomplete was cheating. Might likely be a bug in toolkit/autocomplete...
Blocks: 1071461
Flags: needinfo?(mak77)
Priority: -- → P2
Just realized that this affects plain ol' rightarrow/leftarrow, too -- not just ctrl+rightarrow/leftarrow.  I expect the arrow keys to go forward/back by 1 character, but this bug makes them go all the way to the end.

This trips me up multiple times per day. Adding "dogfood" keyword.
Keywords: dogfood
Summary: Ctrl+rightarrow now jumps to the end of URL, if you've just typed a character. (with Unified Autocomplete enabled) → Leftarrow/rightarrow (and ctrl+rightarrow) jumps to the end of URL, if you've just typed a character. (with Unified Autocomplete enabled)
Summary: Leftarrow/rightarrow (and ctrl+rightarrow) jumps to the end of URL, if you've just typed a character. (with Unified Autocomplete enabled) → Leftarrow/rightarrow (and ctrl+rightarrow) jumps to the end of URL, if Unified Autocomplete dropdown has just appeared
[Tracking Requested - why for this release]: UI regression; breaks the ability to hand-edit URLs with the keyboard pretty badly.

My use cases where I hit this daily:
 - typing a data URL into the URLbar, and then going back and editing it.
 - visiting a news article, and editing it to strip off all the "?arg1=value1&arg2=value2&...&argN=valueN#foobar" junk at the end before sharing it or saving it in Pocket

If at all possible, we should fix this before 41 gets bumped over to Aurora.
Hi Mak, any idea on level of complexity/effort to resolve this bug?  just trying to determine likelihood of quick fix & uplift potential.
Rank: 20
Flags: needinfo?(mak77)
(In reply to Daniel Holbert [:dholbert] from comment #7)
> If at all possible, we should fix this before 41 gets bumped over to Aurora.

(Web developers in particular are likely to run into & be frustrated by this bug, I think. This makes it all the more important that we avoid shipping this to Developer Edition. If this is ends up being too complex to fix quickly, we should seriously consider reverting bug 1168811 (the unified autocomplete pref-flip) before the next merge (in 1.5 weeks), IMO.)
Just want to make sure I understand what is going on here. When I type a character into the url bar and wait for the popup the urlbar now has the first entry selected and the full url in the url bar highlighted. At this point I'd expect pressing right to jump to the end of the selection. So is the problem that you're typing fast and so don't expect the highlighting thing to happen (maybe we wait till you've stopped typing for a certain time before doing that) or is the problem that we do the selection and highlighting at all?
Flags: needinfo?(dholbert)
(In reply to Dave Townsend [:mossop] from comment #10)
> Just want to make sure I understand what is going on here. When I type a
> character into the url bar and wait for the popup the urlbar now has the
> first entry selected and the full url in the url bar highlighted.

That is not what I see. The full URL in the URL bar is not highlighted in the use-case that I'm talking about. (nor should it be)

I'll post a screencast to demonstrate the issue.
Flags: needinfo?(dholbert)
Here's a screencast. I'm going to edit the URL.

The basic scenario is: I want to edit two different sections of the URL: first "EditMe1", then "EditMe2".

I type some text into EditMe1, and then press rightarrow to try to get over to EditMe2. This takes me to the extreme end of the URL.
data URL used in the screencast:
data:text/html,<div>EditMe1</div>EditMe2</div><div>TextThatDoesNotNeedEditing</div>
Here's a second screencast, showing a "editing an actual URL, and making a typo & trying to fix it, and encountering WTF behavior" use-case.

In this screencast, I:
 1. start out on https://www.mozilla.org/en-US/contribute/stories/
 2. Backspace "en-US", and try to type in "es-MX"
 3. I realize I typed "ec" instead of "es" so I try to shift my cursor back over with the leftarrow so that I can backspace the "c" and replace it with an "s".
BUT, when I press leftarrow, my cursor is mysteriously transported to the end of the URL, nowhere near where was just editing.
Attachment #8623965 - Attachment description: screencast → screencast (trying to edit 2 close-by sections of a data URI)
Tracked for 41, as this regression makes it difficult/impossible to edit long urls and users certainly expect to be able to do that, so we want to fix asap.
(In reply to Marco Bonardo [::mak] (Away June 17-22 - then ping me in Whistler) from comment #5)
> I suspect rather than being a bug in unified complete, this is due to
> completedefaultindex that unified complete uses "correctly" while the
> previous autocomplete was cheating. Might likely be a bug in
> toolkit/autocomplete...

I don't think this is completedefaultindex, disabling it doesn't solve the problem. Looks like as long as the popup is open with a selected item this is the behaviour that autocomplete causes: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/autocomplete/nsAutoCompleteController.cpp#520
Attached patch patch (obsolete) — Splinter Review
I can't see anything useful going on in the autocomplete controller where left, right or home are pressed. It just fills the current selection and jumps you to the end of the text which is generally bad for us. Rather than much around with autocomplete though this just overrides the behaviour for this case. Any reason not do do this before I look at tests?
Attachment #8624361 - Flags: feedback?(markh)
Comment on attachment 8624361 [details] [diff] [review]
patch

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

The problem I see with this patch is that it doesn't close the popup on these arrow keys. This leads to:
* start typing, get results.
* press down-arrow to select one of the entries - its URL is in the edit box.
* press left-arrow to move the cursor.
* press delete to remove some characters.

But instead of deleting characters the still-selected entry in the drop-down is removed - so the history entry you explicitly selected has now been deleted from your history!

A slightly more abstract concern is that ISTM that noone actually understands exactly what HandleKeyNavigation in nsAutoCompleteController.cpp does when handling these keys :/ Eg, there's a comment

> // The pop-up is open and has a selection, take its value

which doesn't really make sense to me when handling VK_LEFT and VK_RIGHT. In bug 1159547 comment 1 Mak mentions we do completion on these arrows. I *think* this is more of a concern for other autocomplete users than it is for the URLBar (apparently "in the middle completion" is a thing - bug 719888 comment 30), but I'm not sure that applies here.

The other thing that these arrow keys may do is adjust mSearchString to the current URLBar value and I'm not sure if/when that's important here - I couldn't reproduce this being a problem with this patch.

So tl;dr:
* autocomplete code is a mess and messily handles edge-cases I don't understand and don't seem to be captured very well in tests.
* but our awesomebar needs to be awesome - and this patch should not impact any other autocomplete users, which is nice.
* so I think we should try your patch with the addition of "close the popup" and get multiple people to manually test-like-hell to try and find any other edge-cases
* land it :)
Attachment #8624361 - Flags: feedback?(markh) → feedback-
Attached patch patchSplinter Review
Adds closing the popup and a test to verify.
Assignee: nobody → dtownsend
Attachment #8624361 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8624963 - Flags: review?(markh)
Comment on attachment 8624963 [details] [diff] [review]
patch

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

I'm still a little scared, but we'd never move at all if we let that stop us ;)
Attachment #8624963 - Flags: review?(markh) → review+
Flags: qe-verify+
QA Contact: andrei.vaida
Flags: needinfo?(mak77)
https://hg.mozilla.org/mozilla-central/rev/3606409b170f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Reproduced the issue with Nightly 41.0a1 (2015-06-12), using the str from Comment 0. 

This is now verified fixed on 41.0a1 (2015-06-28), using Ubuntu 14.04 (x64), Mac OS X 10.9.5 and Windows 8.1 (x64). Selections made via [CTRL] + [←] or [→] keyboard shortcuts are now working as expected.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1272505
See Also: → 1327962
You need to log in before you can comment on or make changes to this bug.