344189 - if browser.search.openintab pref is set to true Firefox opens two identical tabs for each search

Status: RESOLVED FIXED

(Firefox :: Search, defect, P1)

Description

[TimoT]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060709 Firefox/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060709 Firefox/3.0a1

This was caused by the VK_ENTER/VK_RETURN switch from bug 337386

Reproducible: Always

Steps to Reproduce:
1. set browser.search.openintab
2. search something
3. enjoy the extra tab

Comment 1

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

This is due to the search latency hack, we now get two events.

Comment 2

[Strephon Alkhalikoi]

Additional information from Bug 344474.  CTRL-Enter or clicking on the Search button with the mouse only opens up one tab.

Comment 3

[Strephon Alkhalikoi]

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

Comment 4

[MechCommanderRei]

(In reply to comment #3)
> *** Bug 344474 has been marked as a duplicate of this bug. ***
> 
Hmm, that duplicate had been Confirmed, but now this current bug report is still marked New.

I'm seeing the bug in 2.0 Beta 1, btw.

Comment 5

[Bill Gianopoulos [:WG9s]]

(In reply to comment #4)
> Hmm, that duplicate had been Confirmed, but now this current bug report is
> still marked New.

This is what New means.  A bug in New status has been confirmed.

Comment 6

[Mike Connor [:mconnor]]

This is a hidden pref right now, unless someone steps up to fix it, it likely won't be fixed.

Comment 7

[Adam Guthrie]

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

Comment 8

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

Attachment: patch #1

I think the only reason we were seeing bug 337386 was that the input sometimes resulted in stopSearch being called after the latency timer was set, which would cancel the timer and result in the search never ending, which would make the autocomplete controller wait forever. This patch should fix that, but it needs careful testing to ensure that it doesn't regress bug 337386.

Comment 9

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

Comment on attachment 230483 [details] [diff] [review]
patch #1

I'd like to try this patch on the trunk to get some widespread testing. In my testing, I noticed that the "search bar doesn't work with enter" bug could be caused by the search bar binding calling stopSearch after an input event, which would cancel the load and make the controller wait indefinitely for the search to return. This fixes that scenario by not cancelling the timer from stopSearch.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 230483 [details] [diff] [review]
patch #1

Seems like a good answer!  Let's get this on trunk and see how it works.

Comment 11

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

mozilla/browser/components/search/content/search.xml 	1.82

Comment 12

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

Oops, I forgot to check in the nsSearchSuggestions.js part of the patch, I did that now.

mozilla/browser/components/search/nsSearchSuggestions.js 	1.8

Comment 13

[Ger Teunis]

The last couple of days my enter key doesn't work all the time anymore.
About 10% of my searches don't react on enter.

Comment 14

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

Ok, so my patch didn't entirely fix the problem :(

Comment 15

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

I know what the problem is, I will hopefully have a fixed patch shortly.

Comment 16

[hhh]

BTW Gavin, the Enter key issue happens even if browser.search.openintab is set to false.

Comment 17

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

(In reply to comment #16)
> BTW Gavin, the Enter key issue happens even if browser.search.openintab is set
> to false.

Yeah, that's known (by me). :) Hopefully I'll have a new patch for this sometime soon.

Comment 18

[Mike Connor [:mconnor]]

Gavin, any ETA here? two days left!

Comment 19

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

Attachment: patch #2

The last patch didn't help because _reset() nulled out _listener, which made notify() a no-op anyways. This changes _reset to only null out _listener and _formHistoryResult if there is no timer pending, in which case we've either already sent the _formHistoryResult to _listener or haven't started the timer yet. Either way, the timer will clean itself up after it fires, if it's initialized at all, by setting _formHistoryTimer to null before calling _reset().

Comment 20

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

This latest attachment will also fix the regression of bug 337386 on the trunk (caused by the first attachment), in addition to this bug.

Comment 21

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

mozilla/browser/components/search/nsSearchSuggestions.js 	1.9

If anyone sees this bug or bug 337386 on the trunk in a build with this patch, please let me know!

Comment 22

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

Attachment: branch rollup

Comment 23

[Mike Connor [:mconnor]]

Comment on attachment 234184 [details] [diff] [review]
branch rollup

a=mconnor on behalf of drivers for 1.8 branch checkin

Comment 24

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

mozilla/browser/components/search/nsSearchSuggestions.js 	1.1.2.8
mozilla/browser/components/search/content/search.xml 	1.37.2.54