335602 - New engines cannot be added to search box

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Charlie Nolan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2

With 1.5.0.2 I am completely unable to add new search engines to the dropdown list of the search box through the Add Engines page.  I have tried several of the engines available, including del.ici.ous and Wikipedia.  I have tried them even after creating a new profile with no luck, a browser restart fails to have the new engine(s) show up in the dropdown list.

Reproducible: Always

Steps to Reproduce:
1. (optional) Create a new profile, launch firefox under it.
2. On the search bar, use the dropdown to visit the Add Engines page.
3. Add one or more of the engines from that page (https://addons.mozilla.org/search-engines.php), accepting the prompt given.
4. Restart the browser.
Actual Results:  
No new search engine(s) are added.

Expected Results:  
The chosen search engine(s) are added.

Comment 1

[whatteaux]

I'm also seeing this bug, on 1.5.0.3 on Win XP. Turning off AdBlock doesn't help. Nor does turning off Java. Nor does turning Javascript off and on again (with all options enabled).

Comment 2

[Charlie Nolan]

I saw the bug on two different computers, both XP.  On one, upgrading to 1.5.0.3 fixed it.  I haven't had a chance to try on the other yet.  I was assuming it got incidentally fixed by .3, but whatteaux's reply seems to disprove that theory.

Comment 3

[Charlie Nolan]

[Whoops, version is 1.5.0.x, not 1.0.]

Comment 4

[Christopher White]

The Error Console is showing JavaScript errors when you click on a link to add a search engine. Still doing it on 1.5.0.3.

[Exception... "Not enough arguments [nsIPromptService.alert]" nsresult:
"0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame ::
file:///C:/PROGRA~1/MOZILL~1/components/nsSidebar.js :: anonymous :: line 164"
data: no]

uncaught exception: [Exception... "Not enough arguments
[nsIPromptService.alert]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"
location: "JS frame :: file:///C:/PROGRA~1/MOZILL~1/components/nsSidebar.js ::
anonymous :: line 164" data: no]

Comment 5

[Charlie Nolan]

I checked up on what Christopher spotted in the console, and I'm getting the same messages (1.5.0.3).  Since then I'm not even getting a question pop-up, making me wonder if the earlier success was a first-run-only result.

Comment 6

[Christopher White]

The method they're calling to show an alert when it catches an exception isn't being passed all of the arguments it needs. It needs 3 and is only getting 2.
Line 164 of nsSidebar.js:

catch(ex)
{
this.promptService.alert(null, "Failed to add the search engine."); //line 164
}

promptService is an instance of Components.interfaces.nsIPromptService.

Mozilla dev docs show that this method has 3 arguments. The 3rd argument is the message for the alert box. I fixed this by changing the problem line to:

this.promptService.alert(null, "Error", "Failed to add the search engine.");



It's throwing an error when it checks to see if engineURL has http:// at the beginning. The problem is all of the URLs use https :-/ I changed the expressions as shown below:

    try
    {
        // make sure using HTTP (for both engine as well as icon URLs)

        if (engineURL.search(/^http:\/\//i) == -1)
        {
            debug ("must use HTTP to fetch search engine file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }
        
        if (iconURL.search(/^http:\/\//i) == -1)
        {
            debug ("must use HTTP to fetch search icon file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

        // make sure engineURL refers to a .src file
        if (engineURL.search(/\.src$/i) == -1)
        {
            debug ("engineURL doesn't reference a .src file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

        // make sure iconURL refers to a .gif/.jpg/.jpeg/.png file
        if (iconURL.search(/\.(gif|jpg|jpeg|png)$/i) == -1)
        {
            debug ("iconURL doesn't reference a supported image file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }
    }
    catch(ex)
    {
        this.promptService.alert(null, "Error", "Failed to add the search engine.");
        throw Components.results.NS_ERROR_INVALID_ARG;
    }

Problem solved! :)

Comment 7

[Christopher White]

I just realized that I didn't post the fixed code. OOPS! Here it is:


    try
    {
        // make sure using HTTPS (for both engine as well as icon URLs)

        if (engineURL.search(/^https:\/\//i) == -1)
        {
            debug ("must use HTTPS to fetch search engine file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

        if (iconURL.search(/^https:\/\//i) == -1)
        {
            debug ("must use HTTPS to fetch search icon file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

        // make sure engineURL refers to a .src file
        if (engineURL.search(/\.src$/i) == -1)
        {
            debug ("engineURL doesn't reference a .src file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }

        // make sure iconURL refers to a .gif/.jpg/.jpeg/.png file
        if (iconURL.search(/\.(gif|jpg|jpeg|png)$/i) == -1)
        {
            debug ("iconURL doesn't reference a supported image file");
            throw Components.results.NS_ERROR_INVALID_ARG;
        }
    }
    catch(ex)
    {
        this.promptService.alert(null, "Error", "Failed to add the search
engine.");
        throw Components.results.NS_ERROR_INVALID_ARG;
    }

Comment 8

[Christopher White]

Attachment: nsSidebar.js

Replace C:\Program Files\Mozilla Firefox\components\nsSidebar.js with this one.

Comment 9

[Christopher White]

Attachment: Fixed nsSidebar.js

Comment 10

[Christopher White]

Posted new nsSidebar.js. Looks for http:// AND https:// in the icon and URL strings.

Comment 11

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

The bug you're patching is bug 268370. This bug is pretty likely caused by localStore.rdf corruption. See http://kb.mozillazine.org/Search_engines_disappear_from_Search_Bar .

Comment 12

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

This doesn't apply to the new search service (landed on branch and trunk), so marking WORKSFORME. People who see problems should follow the instructions at:
http://kb.mozillazine.org/Search_engines_disappear_from_Search_Bar

Comment 13

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

(The underlying cause of the problem was fixed in bug 307558. People using 1.5.0.2 will no longer have their localStore.rdf corrupted, but once it is corrupted it will stay that way, so people who are affected need to delete it.)

Comment 14

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

See also bug 335602 comment 12 and bug 335602 comment 13.

Comment 15

[Christopher White]

(In reply to comment #11)
> The bug you're patching is bug 268370. This bug is pretty likely caused by
> localStore.rdf corruption. See
> http://kb.mozillazine.org/Search_engines_disappear_from_Search_Bar .
> 

No, it's not the same bug. The problem has absolutely nothing to do with storing the search engines. The script doesn't even get that far in it's execution.  If you read my replies then it tells you that the URLs on the Mozilla page have https:// in them when the original method is looking for http://. I changed the method so that it accepts both strings. 

Bug 268370 deals with the JS error in the error console.

The bug you mention and the one you link to are completely different issues. This is simply a JavaScript problem and the above patch fixes it.

Comment 16

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

(In reply to comment #15)
> If you read my replies then it tells you that the URLs on the Mozilla page
> have https:// in them when the original method is looking for http://. I
> changed the method so that it accepts both strings.

The method already accepts both strings. "https?" in a JavaScript regular expression matches "http" and "https". If search plugin files on Mozilla's official search engines distribution site were not installable in Firefox, due to a bug in the client code, don't you think someone would have noticed it earlier?

> The bug you mention and the one you link to are completely different issues.
> This is simply a JavaScript problem and the above patch fixes it.

The one I link to is the cause of the "search plugins won't install" and "search plugins disappeared" issues. The cause of the problem was fixed in 1.5.0.2, but people who still have the corrupted files will still see the problem, regardless of the version they're running. None of this is relevant to the search service that will ship with Firefox 2.

Your attachment isn't a patch, it's a modified file. You need to attach a |cvs diff -up8| from a source tree, or a least a |diff -u| of the original and the new file, to show what you're actually changing.

Comment 17

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

Charlie: I apologize, you were right. I was looking at a more recent version of the file. The Mozilla site was recently changed to use https:// links, and this caused engine installation in 1.5.0.x builds to break, since the patch for bug 267487 didn't land on the 1.8.0 branch. We're going to revert the change to the site so that installation works again. I don't know whether 1.5.0.x builds will be fixed, but that can be tracked in bug 267487.

Comment 18

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

(In reply to comment #17)
> Charlie: I apologize, you were right.

I apologize again, I meant Christopher :)

Comment 19

[Christopher White]

Attachment: Working fix

Comment 20

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

(In reply to comment #19)
> Created an attachment (id=220676) [edit]
> Working fix

Christopher: there's an existing patch in bug 267487, it just hadn't landed on the branches from which the next 1.5.0.x and the 2.0 release come. I've landed it on the 2.0 branch. It will be landed on the 1.5.0.x branch for Firefox 1.5.0.4 if it gets approval.

Comment 21

[Michael Morgan [:morgamic]]

This has been committed and should be resolved (on addons.mozilla.org) in the next 24 hours.

Comment 22

[Charlie Nolan]

I see you guys were busy since I last dropped by.  Nice job getting it fixed!  The workaround on addons has that working again.

Thanks much!
-Charlie Nolan