206464 - Search Sidebar does not remember chosen search engine in advanced mode

Status: RESOLVED FIXED

(SeaMonkey :: Search, defect)

Description

[Jakub Narebski]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4b) Gecko/20030507

The Search sidebar does not remember chosen search engine in advanced search
mode (the mode in which you can divide search engines into categories, and
choose more than one search engine to do the search). It worked in Mozilla rv:
1.3 (Linux/X11). 

Reproducible: Always

Steps to Reproduce:
1.Change search mode to advanced.
2.Choose one category and select one of engines (check one of checkboxes)
3.Select other category or select other sidebar
4.Go back to the previous category/the search sidebar

Actual Results:  
All checkboxes are unchecked. No search engine is chosen.

Expected Results:  
The selection of search engines in categories should be persistent (as it were
in Mozila rv. 1.3).

Comment 1

[Kai de Leeuw]

Seeing this in a recent nightly build (2003082404), but not in 1.4.

Comment 2

[Kai de Leeuw]

This regression from 1.4 is still in the latest nightly, hence it is in 1.5RC2
too. There is been low activity in this bug, is it because this bug is a dupe?

This is a very visible bug for the user in my opionion so it should be fixed in
a release version. Asking for permission to set this as a blocker for 1.5.

Comment 3

[[e-mail]]

This is a regression from bug 190020 - the search panel tries to create invalid
resources which used to work but no longer does.

Comment 4

[[e-mail]]

Attachment: Proposed patch

Comment 5

[Jan Varga [:janv]]

Comment on attachment 132539 [details] [diff] [review]
Proposed patch

>@@ -411,14 +411,9 @@ function SearchPanelStartup()
>   if (catDS) {
>     catDS = catDS.QueryInterface(nsIRDFDataSource);
>     categoryList.database.AddDataSource(catDS);
>-    var ref = categoryList.getAttribute("ref");
>-    if (ref)
>-      categoryList.setAttribute("ref", ref);
>     var engineList = document.getElementById("searchengines");
>     engineList.database.AddDataSource(catDS);
>-    ref = engineList.getAttribute("ref");
>-    if (ref)
>-      engineList.setAttribute("ref", ref);
>+    engineList.addEventListener("CheckboxStateChange", saveEngine, false);

nice cleanup, just make sure that you don't need engineList.builder.rebuild()
here

Comment 6

[Kai de Leeuw]

Bug 215924, bug 218434, bug 213828, bug 208894, bug 187293 seems to be dupes of
this. 

Am I right? Please correct me if I am wrong, and if they are dupes, please mark
them as such, because I don't have such rights myself.

Bug 212703 is another ugly regression from 1.4 Is it related to when bug 206464
regressed?

If I am totally wrong here, then sorry for the bug spam.

Comment 7

[[e-mail]]

As far as I can tell those bugs don't directly relate to advanced mode although
they may still have the same root cause [RDF.GetResource("", true) failing].

Comment 8

[[e-mail]]

Attachment: Addressed review comment

Actually this change turns out to be the fix to bug 212703

Comment 9

[[e-mail]]

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

Comment 10

[Jan Varga [:janv]]

Comment on attachment 132603 [details] [diff] [review]
Addressed review comment

r=varga

Comment 11

[rbs]

Comment on attachment 132603 [details] [diff] [review]
Addressed review comment

passing your request to [e-mail]

Comment 12

[Alec Flett]

Comment on attachment 132603 [details] [diff] [review]
Addressed review comment

sr=alecf

Comment 13

[[e-mail]]

Fix checked in.

Comment 14

[Jakub Narebski]

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