46645 - Internet Search Pref checkbox for Opening sidebar has no effect

Status: VERIFIED FIXED

(SeaMonkey :: Search, defect, P1)

Description

[Josh Stompro]

Using cvs build ( Thu Jul 27 06:28:12 CDT 2000) I have noticed that the checkbox
under edit->preferences->navigator->internet search (Search Results) with the
label "Open the Internet Search My Sidebar panel when search results are
available" has no effect, whenever I go to google and search for something the
sidebar pops up no matter if that checkbox is activated or not.

my configure settings
./configure --enable-ldap --disable-debug --enable-optimize
--x-includes=/usr/X11R6.4/include --x-libraries=/usr/X11R6.4/lib

running 2.2.16 Debian frozen

Comment 1

[gabriel]

Confirmed this under Linux build 2000072408. If the sidebar is open but
minimised, it pops open on a Search, even when the pref is not checked.

Reporter, have you tried unchecking 'My Sidebar' in the View menu ?

Comment 2

[Jeffrey Baker]

Confirmed on Linux 2000072709.  The only workaround is to close the sidebar
entirely (view->sidebar).

Comment 3

[sairuh (rarely reading bugmail)]

btw, after you change the pref, is it written to your prefs.js? if it is, 
methinks this belongs to the Search (or, Sidebar?) component. if it is a problem 
with the pref not being written properly, bounce it back to me.

Comment 4

[Josh Stompro]

Linux Build ID: 2000072805
With a new prefs.js file when I first go into pref->netscape->internet search
and set the engine to google and select the open sidebar button, no variables
get set for the sidebar in the prefs.js.  
From there if I go to google and search the sidebar opens.
If i go back into prefs-netscape-internet search google will not be shown as the
default, and the checkbox will not be set. now if you select then deselect the
checkbox and look at your prefs.js you will see a variable
user_pref("browser.search.opensidebarsearchpanel", false)
close the prefs, minimize the sidebar
go to google again, search and it opens up the sidebar again.
Now if you go view->my sidebar and turn if off, then search again in google it
pops up again.  Now if you minimize the sidebar then turn it off and search it
will pop up the sidebar in its minimized state. If you search on google from
this state it will not pop up all the way, but if you maximize then minimize the
sidebar and search it will pop open again.  
If you go into the prefs->netscape->internet search and check the box for the
popup and then look at you prefs.js the browser.search.opensidebarsearchpanel
variable is gone.  
I guess the prefs are not being set right and the sidebar search isn't reading
it right.  Should this be split into two bugs?
Josh

Comment 5

[Shawn Fumo]

(First time posting a bug comment, so hope this works ok)

I'm running Win98 with Build: 2000091920

The search panel is always popping up if I do a manual search at Google or
netscape or whichever.

Even if I uncheck My Sidebar entirely, it still comes up.

I have the preference for opening the sidebar deselected, and my prefs.js has
the folling entries:

user_pref("browser.search.defaultengine",
"engine://C%3A%5CPROGRAM%20FILES%5CMOZILLAM18%5CBIN%5Csearchplugins%5Cgoogle.src");
user_pref("browser.search.opensidebarsearchpanel", false);

Hope that helps!

Comment 6

[Stephan Niemz]

    I'm also seeing this, both on WinNT (build 2000092420) and on Linux (recent
build from CVS). (Could someone with the appropriate privileges please change
Platform/OS to all/all?)

    It would be nice if someone could fix this ... I always have to close the
sidebar after doing some Google search. Adding myself to the Cc list.

Comment 7

[Claudius Gayle]

damn, how has this been broken for so long? Oops. This used to work (regression)
and is screwed up on all platforms (All/All), even though my mac seems to have other
problems, this feature is kindof a big deal and I don't think anyone want's it to be this
obtrusive/annoying.
Sairuh, i'm not convinced there isn't some prefs goofyness going one here as well.

Comment 8

[don]

Oh hell, this is bad.  What happened to break this?

Comment 9

[Blake Ross]

reassigning to me.  I just looked at this and have a partial fix that may or 
may not be acceptable.  Basically, is it OK if we still load the results in the 
Search panel, but just don't pop out the sidebar and the panel if the user 
unchecks this?  I think this is how it used to work, since the pref is only 
about popping out the sidebar, and the user might change their mind and decide 
to navigate the results via the Search panel.  If this solution is OK, let me 
know and I'll pass it around for review.

Comment 10

[Blake Ross]

Attachment: patch

Comment 11

[Claudius Gayle]

Hmmm,  I'd much rather this be fixed the 'right way' (duh, wouldn't we all). My only concern is
performance - we shouldn't be doing uneccessary work. What about the unresponsive pause
after the page layout and before the search panel finishes? Might we introduce regressions if this
panel generates content that displays nowhere? What sort of state does that leave the sidebar in?
which is the active panel next time i open it?  I dunno, I'm just thinking out loud here, but I
wouldn't mind exploring other fixes.

Comment 12

[Blake Ross]

Upon further consideration, I think this *is* the right fix, considering the 
current wording of the checkbox.  The checkbox only seems to control the opening 
of the sidebar, not whether or not the results load in the sidebar at all.  And 
what happens if the user doesn't want the sidebar popping open everytime he 
searches, but does want the results to be there just in case he wants to 
navigate using it occasionally?  I think this is also how it used to work.  
There is very, very minor regression in performance -- it's still only loading 
the results once, it's just placing them in the tree also.   Right now, there 
*seems* to be a very big performance regression (even a freeze) when searching, 
but that's only because the loading of the results in the panel is stuck in an 
endless loop, meaning it keeps reloading it forever (there's another + bug on 
that).  Once that's fixed, you probably won't even notice the difference.

Re: your questions about the state of the sidebar and the active panel, it 
doesn't affect it at all.  My current patch won't set the active panel (tab) to 
Search if you don't have the pref turned on.  So the sidebar and active panel 
will remain in the same state, but the results will be in the Search panel if 
the user decides to switch to that panel.

In any case, I'd imagine (not being near the code right now) that making the 
results not load in the panel at all if you don't have the pref on wouldn't be 
all that much harder, but then I think we need to change the text of the 
checkbox to something like "Load search results in the Search tab of My 
Sidebar", and for that we'd need i10n approval.

(meanwhile, why does the current wording use the word "panel" instead of "tab", 
and why does it say "Internet Search" instead of just "Search"?  The tab isn't 
called "Internet Search")

Comment 13

[Blake Ross]

hmm.  there's an even easier patch here.  it seems like 
UpdateInternetSearchResults() isn't even necessary at all because it fails in 
the try block.  Removing the entire function (and its associated event 
listener) fixed this and probably got rid of some unnecessary code too, unless 
I'm mistaken.   I emailed rjc about it...

Comment 14

[Blake Ross]

ok, disregard my last comment.

Comment 15

[Blake Ross]

Attachment: new patch

Comment 16

[Blake Ross]

attached a new patch per rjc's suggestion that I just re-retrieve the pref each 
time in UpdateInternetSearchResults() instead of making autoOpenSearchPanel a 
global var (he says this shouldn't cause any noticeable perf/efficiency 
problems since prefs are stored in memory, and notes that this will also 
prevent a rare/race-condition bug from happening).  passing around for r=/sr=...

Comment 17

[don]

I found Robert.  r=rjc.  Now I just need to find you a sr ...

Comment 18

[Scott Collins]

sr=scc

Comment 19

[don]

Fix in hand, reviewed and approved.  PDT, please approve.

Comment 20

[Blake Ross]

checked into trunk.

Comment 21

[Michael La Guardia]

marking rtm++

Comment 22

[sairuh (rarely reading bugmail)]

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

Comment 23

[Claudius Gayle]

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

Comment 24

[Blake Ross]

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

Comment 25

[Blake Ross]

fix checked in to branch.

Comment 26

[Claudius Gayle]

works like a charm VERIFIED Fixed in 2000101608 branch builds AND
VERIFIED Fixed in 2000101608 trunk builds

Comment 27

[Blake Ross]

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

Comment 28

[sairuh (rarely reading bugmail)]

reopening. seems that when i submit a bugzilla query from
http://bugzilla.mozilla.org/query.cgi, the sidebar pops open with the results
even though:

a. my sidebar is closed [off], not merely minimized.
b. i have the pref to turn this off, ie:
user_pref("browser.search.opensidebarsearchpanel", false);

i noticed this using 2001.01.18.08 commercial verif bits on linux
--interestingly, this wasn't a problem with yesterday's build [2001.01.17.08,
comm verif on linux]. any ideas why this regressed recently? is this also a
problem in mozilla and/or other platforms? thx.

Comment 29

[sairuh (rarely reading bugmail)]

how interesting: just tried this out using mozilla [linux, 2001.01.18.15], and
it's not a problem. did something sneak into the commercial tree that could've
caused this? i guess i'll check tomorrow once i'm back in the office.

Comment 30

[Blake Ross]

Remarking fixed per convo. with Sarah on IRC...she's going to check comm builds 
again tomorrow to be sure.

Comment 31

[sairuh (rarely reading bugmail)]

yep, looks good with this morning's [commercial] verification bits on winNT,
linux and macos. re-vrfy.