474470 - Unable to add a search plug-in with window.external.AddSearchProvider()

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Staś Małolepszy :stas]

When you click on a link that is supposed to trigger the window.external.AddSearchProvider() method, nothing happens in Fennec.

I tested on my Mac, alpha2, build from 20081222.

Screenshots and a test case follow.

Comment 1

[Staś Małolepszy :stas]

A test case: http://people.mozilla.com/~stas/bugs/474470/

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Looks like browser/components/sidebar will need to be bundled with Fennec.

Comment 3

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

Indeed, we don't support the window.sidebar or window.external DOM interfaces, since they're implemented in a Firefox-specific file:

browser/components/sidebar/src/nsSidebar.js

We should probably take this opportunity to move these to core code, which should be easy enough.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)
> browser/components/sidebar/src/nsSidebar.js
> 
> We should probably take this opportunity to move these to core code, which
> should be easy enough.

After looking at the implementation, I see some UI code that might be inconsistent with Fennec. The interfaces are already in core code.

Overall, the impl code isn't too bad for Fennec.

Comment 5

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

Some parts of it are app-specific, yeah, but that shouldn't be too hard to split out. We can just fix this in Fennec for now if that's much quicker, but I think it's a good idea to fix it properly at some point anyways.

Comment 6

[Stuart Parmenter]

what is involved here/the overall impact?

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

It's a web content API. Some websites have a button or link "Add a search provider" and this API allows it to work. Just a different way of adding a search provider.

Should be easy to add the code. Simply copy the function from Firefox nsSidebar.js

Comment 8

[Stuart Parmenter]

(In reply to comment #3)
> We should probably take this opportunity to move these to core code, which
> should be easy enough.

yeah, if this is straightforward, we should do that

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: https://addons.mozilla.org/en-US/mobile/search?...

That a screenshot of what happen when I try to install a search engine on AMO Mobile.

I want Fennec to be Mozilla based please! :)

Comment 10

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

Bug 518929 is the right fix, but we probably can't get that in on 1.9.2 at this point. I suppose we have to just copy nsSidebar.js to fennec, and modify it to stub out the microsummary methods.

http://mxr.mozilla.org/mozilla-central/source/browser/components/sidebar/src/nsSidebar.js

Comment 11

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

Attachment: patch

Largely copied from nsSidebar.js in m-c, with some cleanup and the microsummary/panel methods stubbed out (we don't have a microsummary service, and the panel methods are largely unused on the actual web so not worth implementing IMO).

One problem is that the search service handles the prompting, and it includes a "use the search engine now" checkbox which isn't actually useful in our situation (we don't have a "selected engine"). I think I'll fix that by adding a pref we can set to avoid showing that...

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #11)

> One problem is that the search service handles the prompting, and it includes a
> "use the search engine now" checkbox which isn't actually useful in our
> situation (we don't have a "selected engine"). I think I'll fix that by adding
> a pref we can set to avoid showing that...

File a new bug? or just add to this patch?

Comment 13

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

(In reply to comment #12)
> File a new bug? or just add to this patch?

Needs a change to the search service - I filed bug 530533.

Comment 14

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

https://hg.mozilla.org/mobile-browser/rev/ee9693101a64

Comment 15

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

Filed bug 530846 about the prompt checkbox, and bug 530847 about removing this code some day.

Comment 16

[Aakash Desai [:aakashd]]

verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b4pre) Gecko/20091125 Firefox/3.6b4pre Fennec/1.0b6pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.3a1pre) Gecko/20091125 Firefox/3.7a1pre Fennec/1.0b5