32034 - Sidebar Search fails if no navigator windows open.

Status: VERIFIED FIXED

(SeaMonkey :: Search, defect, P3)

Description

[Jean-Francois Ducarroz]

When I try to do a search from the sidebar, either with a debug B1 build or 
debug M15 build on both Mac & Windows, I get the following error:

JavaScript Error: TypeError: parent.content has no properties
URL: chrome://search/content/search-panel.js
Line number: 432

Comment 1

[Jean-Francois Ducarroz]

Ok, I narrow it down. It's append only when you use it from the mail 3-panes 
window. Also, the name of the tab in the mail window is "Search" when it's 
"Search Results" in the browser.

Comment 2

[matt]

.

Comment 3

[don]

Ooops.  Ben, should we fix this for beta 1?

Comment 4

[Phil Peterson]

PDT- for sidebar search in mail. Would be PDT+ for the browser

Comment 5

[Ben Goodger (use ben at mozilla dot org for email)]

Change to PDT + made by Jar

small safe fix in hand, will open search result in new window :-)

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

fix checked in to branch and trunk

Comment 7

[Claudius Gayle]

When I enter a string and click Search in the mail sidebar nothing happens. I tried the beta candidate 2000032006 build on 
WinNT. What was supposed to happen? Should I only get search results in the sidepanel or do they load in the browser window 
too? Reopening.

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

rjc, you break it, you bought it ;) 

Comment 9

[Ben Goodger (use ben at mozilla dot org for email)]

oops didnt read the 'beta1branch' bit.

accepting.. although I think rjc broke this in the tip though ;) 

Comment 10

[Ben Goodger (use ben at mozilla dot org for email)]

ok, I've hacked on this for about a half hour, and I dont think this can be 
easily fixed for beta1, a complete fix for this would be large enough to not 
warrant fixing it at all, and instead pushing back to my rewrite of this code 
post beta1.

Basically the sidebar code makes many assumptions about the presence of an 
internet results pane (a relic from the searchwindow days, translated across 
into the browser) all of which have to be patched or modified, new functions for 
tracking the current results window written, and all thoroughly tested.

does this really kill grandma messily? will she be searching from the search 
panel in the mail sidebar anyway, which is by default (on my machine at least) 
all scrunched up at the bottom... 

also, rjc didnt break anything, that was the fairies talking to me through my 
diet pepsi. 

clearing PDT+ for reconsideration. 

Comment 11

[Robert John Churchill]

<whee!>

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

relnote per jar. 

Comment 13

[Claudius Gayle]

added following text to relnote bug 29686
"Bug 32034: Searches using the Search panel in MySidebar *from a mail window* will not work. Use the search panel from 
MySidebar in a browser window instead."

Comment 14

[don]

Move to M15 for now ...

Comment 15

[leger]

Putting on PDT- radar for beta1.

Comment 16

[don]

Move to M16 for now ...

Comment 17

[leger]

Putting on [nsbeta2+][6/01] radar.  This work must be done by 06/01 or we may 
pull this for PR2.

Comment 18

[Robert Ginda]

This doesn't seem to be a problem anymore.
Searching in mailnews displays the results in mailnews, clicking on a result
displays the page in a browser window.

Comment 19

[Ben Goodger (use ben at mozilla dot org for email)]

rginda tells me this is working now.

Comment 20

[Claudius Gayle]

Alas, this is not the case. reopening.
On Mac with the 2000052312 builds. Entering a search term and clicking Search does absolutely nothing.
On linux with the 2000052408 builds. Doing the same results in the search results page being displayed in the mail window
but no results are displayed in the sidebar panel.

Surprisingly, doing a search with the browser's sidebar will cause the mail sidebar to load with those results but you can't do your
owb searches with the mail sidebar search panel.

Comment 21

[leger]

Changing from [6/01] to [6/15]

Comment 22

[don]

Move to M19.

Comment 23

[Robert Ginda]

This still worksforme, even on Mac.  Around the time this bug changed there was
an unrelated problem with search, maybe that's what you saw?

Comment 24

[Claudius Gayle]

I'm looking at the 200053008 linux build and I see just what I saw before, namely: with the same build and the same profile on 
the same machine at the same time, the browser search panel works and the mail search panel doesn't. I'd be happy to show 
you if you'd like

Comment 25

[Robert Ginda]

ok, here's the deal.  "parent.content has no properties" is the error you got
when searching from *address book*, searching from mailnews looked to me like it
worked, but it turned out to be the wrong behavior.  I'm attaching a fixed
search-panel.js (not a diff) that can be dropped in your
chrome/packages/core/communicator/content/search/ directory.  This should fix
most of the problems with sidebar search, except for the funky [Stop] button
which doesn't go away until you hover over it.

rjc/ben: can you review?

Comment 26

[Robert Ginda]

Attachment: new search-panel.js file

Comment 27

[Robert John Churchill]

Be sure to merge in search-panel.js from the tip as it contains new changes that 
fix other problems.

Rob, can you attach a diff also?

Comment 28

[Robert Ginda]

Attachment: updated search-panel.js

Comment 29

[Robert Ginda]

Attachment: diff -wu

Comment 30

[Robert Ginda]

Fixed some more formatting problems, removed all of the hard tabs (but will they
stay out?) and merged in rjc's latest changes from the tip.

Attached a diff -wu so you can see first hand how hard I spanked this file.

Comment 31

[Claudius Gayle]

I just dropped rginda's most recent attachement into the 2000060208 Comm opt. build on Win98 and I'm getting no love.

**correction, this works just great now from the borwser, mail, and adbook - for SIMPLE searches. Turns out there are some 
issues for multiple engine searches (MES). Who wants bugs??

we can prolly resolve this one as fixed

Comment 32

[Robert John Churchill]

Rob, can you try some multi-engine searches with your changes?  :^)

Comment 33

[Robert Ginda]

multiple engine searches don't seem to work correctly here either, but this
bustage does not seem to be related to my changes.

Comment 34

[Jim Roskind]

Cleaning up status whiteboard by marking beta2 minus (6/15 has passed)

It sounds like this bug is actually fixed... and should be marked as such RSN so 
that it can be verified.  Has the multi-search stuff been sorted out??

Thanks,

Jim

Comment 35

[Robert Ginda]

The patch hasn't been checked in yet, RJC is going to check in after making
changes to fix multiple search.

Right RJC?

Comment 36

[Robert John Churchill]

Attachment: New search-panel.js

Comment 37

[Robert John Churchill]

Attachment: New internetresults.js

Comment 38

[Robert John Churchill]

Rob, please try out the two new files (search-panel.js & internetresults.js)

Comment 39

[Joseph Elwell]

Just checking to make sure: Searching should open the results in a new browser 
window?

Comment 40

[Robert John Churchill]

Once these fixes are checked in [and currently they are not, due to being marked 
nsbeta2-, as well as the fixes need to be tweaked slightly due to other tree 
changes such as "window.content" now being "window._content"], when a search is 
done from the sidebar, instead of just setting "window._content" to something, 
instead the window mediator is used to find the nearest browser window (or open 
one if there aren't any) and use that.  If the active window IS a browser 
window, its used (so no new window in that case).

Comment 41

[Joseph Elwell]

rjc: ok, I've opened a new bug concerning the fact that a browser isn't opened 
in the event that no browser is already open. bug 44797. I need that one fixed 
to clear an nsbeta2+ bug of mine in bugscape. If it's really just a dupe of this 
bug, then I'll need to request new consideration for nsbeta2+ status.

Comment 42

[Robert John Churchill]

Attachment: Latest search-panel.js (now with window._content, and a few other fixes)

Comment 43

[Robert John Churchill]

Attachment: Latest internetresults.js (now with window._content, and a few other fixes)

Comment 44

[Robert John Churchill]

Ben, care to try out these two latest sets of files (search-panel.js and 
internetresults.js)?

They are rginda's changes, along with other window._content usage changes to get 
them working with the tip, along with a few other small tweaks.

Comment 45

[Ben Goodger (use ben at mozilla dot org for email)]

patches checked in. 

Comment 46

[Claudius Gayle]

Dammit, this isn't fixed now. related bug 44797 was recently fixed, maybe this fix regressed (it's been a while).

trying to do a search from the mail sidebar search panel yields no search attempt, much less results, even
with a single engine search.

All platforms 2000082508 builds

my console tells me :
JavaScript error:
chrome://communicator/content/search/search-panel.js line 627: navWindow._content has no properties

and then it goes on to open up webshells and whatnot as it opens an empty window (bug 44797)

nominating nsbeta3

Comment 47

[verah (gone)]

Nav triage team: It's easy for users to enable search tab in mail window, then 
it doesn't work. [nsbeta3+]

Comment 48

[Ben Goodger (use ben at mozilla dot org for email)]

I would have gotten away with it too if it hadn't been for you meddling kids!

I see this too. Interesting. Taking another look... 

Comment 49

[Ben Goodger (use ben at mozilla dot org for email)]

I've patched it so that this works in the case where there is already another 
navigator window open, however in the other case (only mail or 
non-navigator windows open) window.open() appears to be broken in the respect 
that it no longer returns a handle to the new window, instead returns null. This 
is wrong. (See a js reference for more details). Filing a separate bug. 

Comment 50

[Ben Goodger (use ben at mozilla dot org for email)]

In the case where the only window open is a non-navigator window, and a search 
is performed from the sidebar, I have no way to reach the context of the window 
I open. 

hwnd = open[Dialog](..);

does not return a window handle to a ready-to-go context.

Adding a load listener to the handle does not work in this case. 

I have already spent too much time on this. If you search from a mail window 
with a navigator window open, it will work. Removing the nsbeta3+. Will take a 
closer look after NS6. This one remaining case will have to be release noted for 
6.0. Sorry. 

Comment 51

[Ben Goodger (use ben at mozilla dot org for email)]

updating summary to reflect new conditions. 

Comment 52

[johng]

nav triage team:
reassigning to Bijals since we know it is important to shrimp.  Don's team and
Ben have spent enough time on this - if you want to fix it, probably should get
the window.open command fixed (probably danm on trudelle's team, cc'ing danm).

Comment 53

[verah (gone)]

Nav triage team: See also bug 50894

Comment 54

[bijals (gone)]

Shrimp will be using a different Search tab than this client side Search tab so 
I would keep this bug a minus and future it.

Comment 55

[Michael La Guardia]

Per bijal's last comment, marking nsbeta3-.  Setting milestone to future.

Comment 56

[Gervase Markham [:gerv]]

I have no idea how, but this seems to work now - doing a sidebar search in Mail 
opens a new Nav window, and does the search in there.

So:
It seems unclear to me whether this bug requires either of a "developer" or 
"user" release note for Netscape 6 RTM. If anyone feels it does, can they please 
draft one and then nominate with the relnote-user or relnote-devel strings in 
the Status Whiteboard.

Thanks :-)

Gerv

Comment 57

[Marcia Knous [:marcia]]

Netscape Nav triage team: Marking as fixed. Added Syd to the cc list - if this 
bug is an issue for you, please track separately in Bugscape.

Comment 58

[Claudius Gayle]

Dang, not working with the 2001041004 build on Win98. Entering a search term in the mail search
sidebar panel with no nav window open correctly opens a window and populates the sidepanel with
the search string but the search is never fired off - so consequently no results in the content area
or the sidebar. Reopening.

Comment 59

[Joseph Elwell]

bijals doesn't work here anymore.

Comment 60

[Paul Chen]

nav triage team:

We believe that usage of sidebar search outside of the navigator window is small. 
Pushing out to mozilla1.0

Comment 61

[Samir Gehani]

Moving to mozilla0.99.

Comment 62

[Peter Trudelle]

p3/1.0, nsbeta1+

Comment 63

[Peter Trudelle]

Reassigning 'several bugs at once' to Steve Morse, to level the load better
across the team.

Comment 64

[Claudius Gayle]

marking nsbeta- per adt triage

Comment 65

[Peter Trudelle]

Todd, are you okay with minusing this?  AIM seemed to place a lot of value in a
working sidebar...

Comment 66

[Todd Pringle]

No, I am not.  With autostart of IM at OS boot coming in Mach V, I believe we
have to fix this one or else the Search experience will be broken in standalone.
 Removing the minus and strongly recommending a plus.

Alternatively, someone tell me I am smoking crack and this bug would not affect
standalone IM with no Nav window open.

Comment 67

[Peter Trudelle]

nsbeta1+ per Nav triage team, nav2

Comment 68

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 69

[scottputterman]

adt2 per nav triage

Comment 70

[Peter Trudelle]

->ben

Comment 71

[Blake Ross]

Attachment: patch

Comment 72

[Blake Ross]

-> me

Comment 73

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 81441 [details] [diff] [review]
patch

sr=[e-mail]

Comment 74

[Brian Ryner (not reading)]

Comment on attachment 81441 [details] [diff] [review]
patch

r=bryner

Comment 75

[Blake Ross]

fixed on trunk.

Comment 76

[tao]

will the patch cause a new browser window to open when the SB search happens in
an existing browser window? Also, does this bug happen in 3 pane mail window
only? How about Editor, Addressbook, AIM, etc?

Comment 77

[Jaime Rodriguez, Jr.]

nice work, let's get this one on the RTM train. adt1.0.0-/adt2RTM. 

Comment 78

[Paul Wyskoczka]

Claudius, can you verify this fix on the trunk.  Thx.

Comment 79

[Claudius Gayle]

VERIFIED Fixed with 20020626 trunk builds

Comment 80

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking into the branch.

Comment 81

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 82

[scottputterman]

Please check this into the branch today (or if it's already checked in add the
fixed1.0.1 keyword).  The adt1.0.1+ may be removed if it's not in tomorrow's builds.

Comment 83

[Claudius Gayle]

VERIFIED Fixed with 20020722 branch builds