Closed
Bug 509048
Opened 15 years ago
Closed 15 years ago
hitting enter after a paste into the location bar does nothing
Categories
(Toolkit :: Autocomplete, defect)
Toolkit
Autocomplete
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: dietrich, Assigned: sdwilsh)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
5.87 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
Possibly step #4: after hitting enter, immediately switch to a different tab.
Comment 2•15 years ago
|
||
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•15 years ago
|
Flags: blocking-firefox3.6?
Assignee | ||
Comment 3•15 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•15 years ago
|
Flags: blocking1.9.2?
Updated•15 years ago
|
Flags: blocking1.9.2? → blocking1.9.2+
Assignee | ||
Comment 5•15 years ago
|
||
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•15 years ago
|
Whiteboard: [needs review gavin]
Assignee | ||
Updated•15 years ago
|
Comment 7•15 years ago
|
||
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•15 years ago
|
||
Per irc discussion.
Attachment #394129 -
Attachment is obsolete: true
Attachment #395391 -
Flags: review?(gavin.sharp)
Comment 9•15 years ago
|
||
Isn't this similar to the bugs on AVG products horking the location bar?
Comment 10•15 years ago
|
||
No, this is a specific regression from the async location bar changes, which Shawn has identified and has a patch for.
Updated•15 years ago
|
Attachment #395391 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Updated•15 years ago
|
Whiteboard: [needs review gavin] → [can land]
Reporter | ||
Comment 11•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Updated•15 years ago
|
Flags: in-testsuite?
Assignee | ||
Updated•15 years ago
|
Flags: in-testsuite?
Flags: in-testsuite-
Flags: in-litmus?
Target Milestone: mozilla1.9.2b1 → mozilla1.9.3a1
Assignee | ||
Comment 13•15 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•