509048 - hitting enter after a paste into the location bar does nothing

Status: RESOLVED FIXED

(Toolkit :: Autocomplete, defect)

Description

[Dietrich Ayala (:dietrich)]

i can't reproduce this on-demand, but have hit it enough times that i'm sure it's not user-errror.

STR:
1. open a new tab
2. paste a URL into the location bar
3. hit enter

Expected: page opens

Actual: nothing happens

Comment 1

[Dietrich Ayala (:dietrich)]

Possibly step #4: after hitting enter, immediately switch to a different tab.

Comment 2

[(not currently active) Ted Mielczarek]

I can quite reliably reproduce this:
1) Copy a URL to the clipboard
2) Ctrl+T to open a new tab
3) Ctrl+V to paste the URL into the new tab's location bar
4) Hit enter to load new URL
5) Quickly hit Ctrl+PgUp to switch back to the preceding tab

Expected: new page loads in background
Actual: visible tab gets reloaded(!) (I realized this when I switched back to a tab containing a wiki page with unsaved edits, and it reloaded and lost my edits.)

I've been hitting this frequently, but I don't know when it started. I frequently paste URLs into new tabs and then switch back to another tab, and I've noticed that the background tab winds up still empty with the URL in the location bar.

Comment 3

[Shawn Wilsher :sdwilsh]

Oh this is so much fun.  Have I ever mentioned how much I hate our AutoComplete code?  In nsAutoCompleteController::HandleEnter, we have this little comment:
  // clear the search timer only if we are not searching.
  // if we are searching, EnterMatch() will not handle the enter
  // immediately.  instead, we will handle it on the next result we process
  // but we need the search timer to fire to kick of that search

nsAutoCompleteController::EnterMatch does exactly as the comment says.  If we are searching, it will set mEnterAfterSearch to a non-zero value and then returns NS_OK.

Then, when we get results back on the main thread, in nsAutoCompleteController::ProcessResult we that mEnterAfterSearch is non-zero and call nsAutoCompleteController::StopSearch which calls nsAutoCompleteController::PostSearchCleanup which then calls nsAutoCompleteController::EnterMatch again.  This ends up calling the autocomplete binding's onTextEntered method, which 'fires' a "textentered" event that will call this.handleCommand(param), where param is going to be this.mEnterEvent.   this.handleCommand is on urlbarBindings.xml, which ends up loading the URI.

The problem is that because our results are asynchronous, other events (like changing a tab) can happen before our results come back, and then the contents of the input field of the location bar have changed.

Comment 5

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

This stops the search if we aren't handling a popup selection.  Try server builds in a bit.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 394129 [details] [diff] [review]
v1.0

Don't think this should be conditional on aIsPopupSelection (that's for differentiating clicks vs. keypresses, not popup vs. no-popup), and I think we agreed on IRC to just try removing mEnterAfterSearch completely.

Comment 8

[Shawn Wilsher :sdwilsh]

Attachment: v2.0

Per irc discussion.

Comment 9

[[not reading bugmail]]

Isn't this similar to the bugs on AVG products horking the location bar?

Comment 10

[(not currently active) Ted Mielczarek]

No, this is a specific regression from the async location bar changes, which Shawn has identified and has a patch for.

Comment 11

[Dietrich Ayala (:dietrich)]

http://hg.mozilla.org/mozilla-central/rev/b0be3786ac2d

Comment 13

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/80081b11c0cf