hitting enter after a paste into the location bar does nothing

RESOLVED FIXED in mozilla1.9.3a1

Status

()

RESOLVED FIXED
10 years ago
10 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

10 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

10 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

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

Comment 3

10 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

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

Updated

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

Comment 5

10 years ago
Posted patch v1.0 (obsolete) — Splinter Review
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

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

Updated

10 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

10 years ago
Posted patch v2.0Splinter Review
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

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

Comment 11

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

Updated

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

Updated

10 years ago
Duplicate of this bug: 512987
You need to log in before you can comment on or make changes to this bug.