The default bug view has changed. See this FAQ.

hitting enter after a paste into the location bar does nothing

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Toolkit
Autocomplete
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dietrich, Assigned: sdwilsh)

Tracking

({regression})

Trunk
mozilla1.9.3a1
regression
Points:
---
Bug Flags:
blocking1.9.2 +
in-testsuite -
in-litmus ?

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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
(Reporter)

Comment 1

8 years ago
Possibly step #4: after hitting enter, immediately switch to a different tab.
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.
(Assignee)

Updated

8 years ago
Flags: blocking-firefox3.6?
(Assignee)

Comment 3

8 years ago
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.
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Component: Location Bar and Autocomplete → Autocomplete
Flags: blocking-firefox3.6?
Product: Firefox → Toolkit
QA Contact: location.bar → autocomplete
Target Milestone: --- → mozilla1.9.2b1
(Assignee)

Updated

8 years ago
Flags: blocking1.9.2?
Flags: blocking1.9.2? → blocking1.9.2+
(Assignee)

Updated

8 years ago
Duplicate of this bug: 510016
(Assignee)

Comment 5

8 years ago
Created attachment 394129 [details] [diff] [review]
v1.0

This stops the search if we aren't handling a popup selection.  Try server builds in a bit.
Attachment #394129 - Flags: review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Whiteboard: [needs review gavin]
(Assignee)

Updated

8 years ago
(Assignee)

Updated

8 years ago
Duplicate of this bug: 510416
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.
Attachment #394129 - Flags: review?(gavin.sharp)
(Assignee)

Comment 8

8 years ago
Created attachment 395391 [details] [diff] [review]
v2.0

Per irc discussion.
Attachment #394129 - Attachment is obsolete: true
Attachment #395391 - Flags: review?(gavin.sharp)
Isn't this similar to the bugs on AVG products horking the location bar?
No, this is a specific regression from the async location bar changes, which Shawn has identified and has a patch for.
Attachment #395391 - Flags: review?(gavin.sharp) → review+
(Assignee)

Updated

8 years ago
Whiteboard: [needs review gavin] → [can land]
(Reporter)

Comment 11

8 years ago
http://hg.mozilla.org/mozilla-central/rev/b0be3786ac2d
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Flags: in-testsuite?
(Assignee)

Updated

8 years ago
Flags: in-testsuite?
Flags: in-testsuite-
Flags: in-litmus?
Target Milestone: mozilla1.9.2b1 → mozilla1.9.3a1
(Assignee)

Updated

8 years ago
Duplicate of this bug: 512987
(Assignee)

Comment 13

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/80081b11c0cf
You need to log in before you can comment on or make changes to this bug.