339697 - Search bar opens new tab if enter is pressed before search suggest is finished getting terms

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[pile0nades]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060530 BonEcho/2.0a3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060530 BonEcho/2.0a3

If I hit enter immediately after typing in the search box, it opens the results in a new tab. If I wait a second of two it opens the results in the same tab. I think this has to do with the search suggest feature asking Google for related terms or something. People with fast connections might have problems confirming this because it finishes too fast.

Reproducible: Always

Steps to Reproduce:
1. set browser.search.openintab to false
2. type in the search box and quickly press enter.


Actual Results:  
It opens a new tab with the results.

Expected Results:  
It should open in the same tab.

Comment 1

[Ria Klaassen (not reading all bugmail)]

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060529 BonEcho/2.0a3
When I type vvv faster than Google Suggest can react, it just opens the search in the same tab.

Comment 2

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

I was able to reproduce by typing and pressing enter quickly without delay. I looked into this and found that the event we were checking in handleSearchCommand (mEnterEvent, passed from onTextEntered) returned altKey=true even if it wasn't because its mEvent's isAlt was some bogus non-zero value. smaug's says that this is due to the autocomplete binding holding on to mEnterEvent after it has been dispatched. He suggested that copying trunk's DuplicatePrivateData to branch would probably fix it. A work around would be to hold on to an identical event instead of the dispatched event in autocomplete.xml.

This is a branch-only bug.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nsDOMEvent::DuplicatePrivateData from trunk with small changes

This is just copying nsDOMEvent::DuplicatePrivateData from trunk to branch.
The changes needed are the different event target handling, and
copying nsEvent's internalAppFlags and point (those aren't in trunk nsEvent).
So these lines:
+  if (!mTarget) {
+    // GetTarget sets mTarget
+    nsCOMPtr<nsIDOMEventTarget> dummy;
+    GetTarget(getter_AddRefs(dummy));
+  }
...
+  newEvent->internalAppFlags       = mEvent->internalAppFlags;
...
+  newEvent->point                  = mEvent->point;

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 224056 [details] [diff] [review]
nsDOMEvent::DuplicatePrivateData from trunk with small changes

still seeing some leaks from bug 339774. Cancelling
reviews until I've fixed those.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: with memleak fixes

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 224421 [details] [diff] [review]
with memleak fixes

r+sr=jst

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 224421 [details] [diff] [review]
with memleak fixes

hmm, I forgot to ask approval

Comment 8

[Robert Strong (they/them - no direct email)]

I have the pref set to always open in a new tab and as of late it will often open in the same tab. :(

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Checked in to branch.
Robert, I can't reproduce your problem with or without this patch.

Comment 10

[Robert Strong (they/them - no direct email)]

I haven't been able to come up with reliable steps to reproduce so I haven't filed.

Comment 11

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

*** Bug 341591 has been marked as a duplicate of this bug. ***