Installation of search engines fails in Firefox < 4.*

VERIFIED FIXED in 5.12.10

Status

defect
--
major
VERIFIED FIXED
9 years ago
4 years ago

People

(Reporter: krupa.mozbugs, Assigned: andy+bugzilla)

Tracking

({regression})

Dependency tree / graph
Bug Flags:
in-testsuite ?
in-litmus ?

Details

()

Attachments

(1 attachment)

steps to reproduce:
1. Load https://addons.mozilla.org/en-US/firefox/search-tools/bookmarks
2. Click on "Add to Firefox" for any of the listed add-ons

observed behavior:
Install Error
This search engine isn't supported by Firefox and can't be installed.

Note: works fine in FF 4
Posted image install error
We can't reproduce this in 3.6.13.  What add-on did you try to install?
I can reproduce in 3.6.13 (Windows), and I get:

Error: Invalid argument passed to window.sidebar.addSearchEngine: Unsupported search icon URL.
Source File: file:///C:/Program%20Files/Mozilla%20Firefox/components/nsSidebar.js
Line: 112

In nsSidebar.prototype.validateSearchEngine =
function (engineURL, iconURL), the part that's failing is:

// Make sure we're using HTTP, HTTPS, or FTP and refering to a
    // .gif/.jpg/.jpeg/.png/.ico file for the icon.
    if (iconURL &&
        ! /^(https?|ftp):\/\/.+\.(gif|jpg|jpeg|png|ico)$/i.test(iconURL))
      throw "Unsupported search icon URL.";

Mossop, any ideas?
OK; works fine from top-level, such as https://addons.mozilla.org/en-US/firefox/search-tools/, (prod/preview/next), and fails with the above error, on 3.6.* with the sub-categories (comment 0).  If I knew how to use Firebug, I'd tell you what the value of iconURL is, and why it's failing i.test
The most recent thing messing with this was the switch to putting icons on the static.amn CDN.  I don't know why this would work for top level pages, and fx4 though.

Sending a cache busting string (?blah on the end of the URL) wouldn't match that regex - that could be what's happening.  I don't have 3.6 here to test.
This seems like a critical bug that should have been fixed Friday, unless I'm misunderstanding something?
(In reply to comment #6)
> This seems like a critical bug that should have been fixed Friday, unless I'm
> misunderstanding something?

It's important.  Mossop - can you tell us why this works in FF4 but not 3.6?
We loosened the iconURL validity check in 4.0, in bug 550290. If comment 5 is correct (?blah causing the failure), then I guess this is invalid?
Thanks Gavin.  It's still valid, since we still support 3.6.  It makes an unfortunately specific case in our caching strategy though.
This isn't the first time we've run into the question mark breaking InstallTrigger either, unfortunately. Though last time it was extensions I think.
Actually, I don't know how this would be our fault.  Search engines either use data URLs or href's in their opensearch XML, but neither of them should be coming from AMO.

QA:  What add-on are you failing to install, specifically?
(In reply to comment #11)

> QA:  What add-on are you failing to install, specifically?

All/any of them in a sub-category of /search-tools/.  Another data point is that when I try to install from /search-tools/ on preview/allizom, I get the requisite "Firefox prevented this site (addons.allizom.org) from asking you to install software on your computer" info-bar, while I don't on /search-tools/bookmarks, e.g.

Try: SearchGeek for Firefox or Google.com.tr, or any of those; again, other categories fail too, like News & Blogs (so, Bing engine fails to install).
in-testsuite? and in-litmus? -- we shouldn't regress this again.  Not sure if it's best-addressed by a Django unittest or if we could whip something up in Selenium, at least.
Flags: in-testsuite?
Flags: in-litmus?
It looks like it doesn't even request the icon in the opensearch package, it just loads directly off the page (doesn't make any more http requests after clicking).

So yeah, it's the cache buster.
We call this to add a search engine:

        window.external.AddSearchProvider(url);

That url is something like https://addons.mozilla.org/firefox/downloads/latest/10772/addon-10772-latest.xml?src=category (whatever the install button's href is).

I don't see how any image URLs from AMO would get in there.
I think the change was here:

https://github.com/jbalogh/zamboni/commit/c865421e#L18L11
Blocks: 634429
Assignee: nobody → amckay
We have the same problem in our Mozmill test for the SearchGeek search engine. See bug 634429.
And this happens also in Firefox 3.5.
Summary: Install of search tools fails in FF 3.6.* → Installation of search engines fails in Firefox 3.6 and 3.5
Summary: Installation of search engines fails in Firefox 3.6 and 3.5 → Installation of search engines fails in Firefox < 4.*
https://github.com/jbalogh/zamboni/commit/66920051
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
verified on 3.6.13 and 3.5.
Status: RESOLVED → VERIFIED
Has this already been pushed live? Given our latest Mozmill tests for 3.6.14 build 3 it's still broken:

http://mozmill-release.brasstacks.mozilla.com/#/general/report/cf236ad19b037e87fe0fe25cddfccbef
This is fixed in preview and was pushed live in 5.12.9.4. But, I can reproduce the issue on prod.
Thanks Krupa => Reopening because it is still an issue on AMO.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
What I forgot to mention, I can't see this behavior on allizom. Do we simply have forgotten to push this patch to production?
Yep, this was fixed last night.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Works now on AMO. Marking as verified fixed.
Status: RESOLVED → VERIFIED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.