Closed Bug 55277 Opened 24 years ago Closed 24 years ago

Multiple engine search within a category (not All Engines) is broken

Categories

(SeaMonkey :: Search, defect, P2)

Product:
SeaMonkey
Component:
Search
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: cmaximus, Assigned: mozilla)

Details

(Keywords: regression, Whiteboard: [rtm++][Fix in Hand][Fixed on trunk]) 

***Overview Description: 
	Doing a multiple engine search (MES) is hella broken for searches where you've used the 
'within' dropdown to narrow down the list of search engines to a particular category.
  

***Steps to Reproduce: 

1) Advanced search mode. Search|My Sidebar search panel|Advanced.

2) enter a search term.

3) Select the 'Web' category for example.

4) Select two or more search engines.

5) Click 'Search' button.


***Actual Results: 

No sidebar search results. No MES results in the content window. Moreover, the content
window stops redrawing. Pop down a menu or move the window around to see this.

***Expected Results: 

The fancy little MES results page should appear - like it does when you do a search from the
'All Engines' menuitem.

***Build Date & Platform Info: 

2000100408 branch builds

***Additional Information:
thanks to jrgm for uncovering this bug. Here is a comparison of the console output from
a successful search vs. the broken search. The difference lies in use of a file path for the
first but the ill-fated urn: for the second.(right after the engine=)

Successful 'All Engines' search:
chrome://communicator/content/search/internetresults.xul?internetsearch:engine=engine://
E%3A%5CProgram%20Files%5CNetscapeOct04-MN6%5CNetscape%206%5Csearchplugins%5CMusic_Artist.src&engine=engine://
E%3A%5CProgram%20Files%5CNetscapeOct04-MN6%5CNetscape%206%5Csearchplugins%5CAOLSearch.src&text=car

Failed 'Web' search:
chrome://communicator/content/search/internetresults.xul?internetsearch:engine=NC:SearchCategory?engine=
urn:search:engine:ClassifiedPlus.src&engine=NC:SearchCategory?engine=urn:search:engine:ShopNetscape.src&text=car
keyword love. (rtm, regression).
Keywords: regression, rtm
The URN is question looks OK.

This is something bad happening with chrome loading and passing options along
with the chrome: URL.  The fact that the content area stop redrawing indicates
that something has really gone south.

First stop:  a XUL expert on Peter's team, I guess.
Assignee: rjc → trudelle
ummm, but how come it uses the URN when the other search doesn't? Is that supposed to
be?

There is more than one type of search URN... basically, relative and absolute.  
This stems from the Mac where under Mac OS 9, Sherlock files can come from 
multiple locations:  Mozilla's "Search Plugins" folder, various directories 
inside of the "System Folder", etc.

Irregardless of platform, as long as either resolves to an existing Sherlock 
file, it is valid.
->hyatt for triage
Assignee: trudelle → hyatt
This is way too obscure to consider.  Feel free to escalate/reassign if you
disagree.  rtm-/future
Whiteboard: [rtm-]
Target Milestone: --- → Future
Robert, does anyone have a fix or patch?  It would be great to get this fixed.

However, if we don't have a known fix at this late date, we probably have 2
realistic (but sucky) options that don't put the schedule at risk:

1) Ship as is.  The user will only run into trouble when they choose the web
category (the only category with more then on engine by default) and choose more
then one engine, or if they add engines to the categories then use more then one
at a time.

2) Ship with no categories, not even "web".  So the drop down choices by default
would say "All Engines" and "Edit Categories ..." and that's it.

I think I would choose option 1 (assuming that fixing it is not an option).

If we did have a fix or a patch, then I think we should consider using it, but
since we are about to freeze the branch for everything except stop ship bugs, we
probably are stuck with this damn annoying bug for our first version.

This has something to do with the fact that in the second (failed) url given, 
the engines are delimited by question marks instead of ampersands.  The only 
question mark in the uri should be right after the file extension.  If I 
replace all but the first question mark in the uri with "&" 's, the multiple 
search results window loads at least, but the results still don't load. 
Investigating...
Yes, as Robert said, it is indeed a problem with the urn.  Is a valid 
workaround to just use the engine:// urls as All does?
So meanwhile, I'm seeing a crash in JS3250.DLL in my trunk build after 
searching in All, right clicking on an item in the search results tree, and 
then closing the browser.  Anyone else...?
Not seeing that crash in a trunk opt. build from ~2am this morning on win2k
What is the default category with a new profile?  I think it was "Web", but with
this problem, we probably ought to change it to "All Engines".

*** Bug 56882 has been marked as a duplicate of this bug. ***
The default for a new profile is indeed "Web".

Um, but in 2000101608 win32, 2000101509 linux, and 2000101608 mac branch 
builds, the Web category is now _empty_, as is the Music category, and the 
Tech category has gone AWOL. Er, um ...
jrgm: You made need to create a new profile (as the list of search engines 
changed)

That's what I was doing ... creating a new profile. What gets deposited in 
that new profile looks nothing like what is located in .\defaults\profile
on Windows (I know that Linux and mac are screwed up too, but haven't looked
at the RDF files). 

I'll file a bug and attach a copy (i.e. this is a separate bug from the one 
that started this bug report). 
filed bug 56933, on the other point.
Don't worry about the tech category. It was supposed to be pulled.
I'm taking this bug back.  While I have no idea what's confusing XUL so much to 
get the browser into a horked state, I did come up with a quick patch to work 
around the problem... basically, just escape() and unescape() the URL in 
question.


Index: mozilla/xpfe/components/search/resources/internetresults.js
===================================================================
87c87
<   resultsTree.setAttribute("ref", aSearchURL);
---
>   resultsTree.setAttribute("ref", unescape(aSearchURL));


Index: mozilla/xpfe/components/search/resources/search-panel.js
===================================================================
793c793
<         loadURLInContent("chrome://communicator/content/search/
internetresults.xul?" + searchURL);
---
>         loadURLInContent("chrome://communicator/content/search/
internetresults.xul?" + escape(searchURL));
Assignee: hyatt → rjc
r=danm. rjc has showed me flow of control between these two bits and convinced me 
it's a fine change.

Y'know, I thought I checked whether bug 56529 was a factor, but I may have 
missed something. At any rate, you should be aware that Location.search is 
currently doing an unescape of the query string for you. You've probably
insulated yourself against this by doing this escape/unescape, and probably
insulated yourself should this property be changed to do the right thing, 
which is to return the "raw" query string, as every other browser has always
done.
this patch looks good and tests perfectly. a=ben. 


Time to put pressure on PDT to accept this, otherwise search (the 2nd biggest 
user activity after generic browser) is fairly broken... so rtm+'ing it.
Status: NEW → ASSIGNED
Whiteboard: [rtm-] → [rtm+][Fix in Hand]
Whiteboard: [rtm+][Fix in Hand] → [rtm+][Fix in Hand][Fixed on trunk]
Priority: P3 → P2
I think we agreed rtm++ on this bug in PDT today, but it's not marked. Making it so.
Whiteboard: [rtm+][Fix in Hand][Fixed on trunk] → [rtm++][Fix in Hand][Fixed on trunk]
Fixed on both trunk and branch now.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
VERIFIED Fixed with 2000102509 branch builds.
adding vtrunk keyword sothis can be verified on trunk builds
Keywords: vtrunk
mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]
Status: RESOLVED → VERIFIED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.