Make window.external.AddSearchProvider a dummy function

RESOLVED FIXED in Firefox 65

Status

()

enhancement
RESOLVED FIXED
8 months ago
29 days ago

People

(Reporter: baku, Assigned: baku)

Tracking

(Depends on 1 bug, Blocks 4 bugs, {dev-doc-complete, site-compat})

unspecified
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

8 months ago
https://html.spec.whatwg.org/#external
"The AddSearchProvider() and IsSearchProviderInstalled() methods must do nothing."
Assignee

Comment 1

8 months ago
Let's remove the support for this function just in nightly, as smaug suggested.
Attachment #9021500 - Flags: review?(bugs)
Assignee

Comment 2

8 months ago
The spec wants window.external
Attachment #9021502 - Flags: review?(bugs)
Assignee

Updated

8 months ago
Attachment #9021500 - Attachment description: sidebar.patch → part 1 - dummy window.external methods
Comment on attachment 9021500 [details] [diff] [review]
part 1 - dummy window.external methods

ok, let's try this.
Attachment #9021500 - Flags: review?(bugs) → review+

Updated

8 months ago
Attachment #9021502 - Flags: review?(bugs) → review+
Assignee

Comment 4

8 months ago
Attachment #9021500 - Attachment is obsolete: true
Attachment #9021508 - Flags: review?(bugs)
Assignee

Comment 5

8 months ago
Just a changes in browser.ini which was brone. I guess I can keep the r+.
Assignee

Updated

8 months ago
Attachment #9021508 - Flags: review?(bugs)

Comment 6

8 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23850b9ba24d
Make window.external.AddSearchProvider a dummy function, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/7db2b627cacc
Remove window.sidebar, r=smaug
See Also: → 862147

Comment 7

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/23850b9ba24d
https://hg.mozilla.org/mozilla-central/rev/7db2b627cacc
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
NOOOOOOOOOOOO :( I knew this day would come, but AddSearchProvider is important for my extension: https://addons.mozilla.org/en-US/firefox/addon/add-custom-search-engine/, because Firefox does not provide a reasonable way to add custom search engines.
Assignee

Comment 10

8 months ago
Maybe we can have a webExtension to support this feature. Do you mind to file a bug?
Flags: needinfo?(evilpies)
Assignee

Updated

8 months ago
Blocks: 1346123

Comment 11

8 months ago
AddSearchProvider is required by my extension as well: https://addons.mozilla.org/en-US/firefox/addon/search-engines-helper/
A web extension API to add (and manage) search engines would be very welcome
Blocks: 1504338
(In reply to Andrea Marchesini [:baku] from comment #10)
> Maybe we can have a webExtension to support this feature. Do you mind to
> file a bug?
Opened bug 1504338
No longer blocks: 1504338
Flags: needinfo?(evilpies)
Did you remove all these APIs completely?

If so, we can't do this yet. This breaks all AMO search engines.

Please test AMO engines on nightly.
Assignee

Comment 14

8 months ago
(In reply to Mike Kaply [:mkaply] (on PTO until Nov 5) from comment #13)
> Did you remove all these APIs completely?

No. I just made the exposed window.external methods dummy as the spec says here:

https://html.spec.whatwg.org/#dom-external-addsearchprovider

The code is still there.
Blocks: 1504338
This also breaks installing search engines from https://mycroftproject.com/
Sorry, missed this comment:

> Let's remove the support for this function just in nightly, as smaug suggested.

There are a lot of other things that need to happen for this function to be completely removed.

Side note, I had originally opened another bug for this work:

https://bugzilla.mozilla.org/show_bug.cgi?id=1377213

The biggest blocker to putting this in release is AMO.
Blocks: 1377213
OK, so I've had a go at documenting this, unusual as it is ;-)

I added a quick page covering external:

https://developer.mozilla.org/en-US/docs/Web/API/Window/external

I don't think we need anything more in-depth, as it is obsolete and the functions do nothing.

I've also submitted a PR to add/update compat data for these features:
https://github.com/mdn/browser-compat-data/pull/3111

I've noted that the external methods do nothing, and that window.sidebar has been put behind a pref in Nightly only.

I also added a note to the FX65 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/65#APIs

Let me know if this all sounds OK, or if I've got something wrong here. Thanks!
Flags: needinfo?(amarchesini)
Assignee

Comment 18

7 months ago
> Let me know if this all sounds OK, or if I've got something wrong here.

So far, all of this happens in nightly only because we want to collect regressions. I'm planning to disable Window.external for 66.
Flags: needinfo?(amarchesini)
> So far, all of this happens in nightly only because we want to collect regressions.

OK, thanks. In that case I've removed the rel note and the nightly-only stuff from the compat data for now. I'll add them back in when it lands in release.

> I'm planning to disable Window.external for 66.

Cool; add dev-doc-needed to the relevant bugs and I'll make sure the docs are sorted!
This is a very major change that will break AMO and other sites. It can't be put in without deprecation and a plan to solve those other sites.
Assignee

Updated

7 months ago
Blocks: 1509061

Updated

7 months ago
No longer blocks: 1504338, 1509061
Depends on: 1504338, 1509061, 1106626
Am I missing something here (I haven't been following browser dev for some time)
Is this just pandering to a standard without an alternative or am I missing a suggested alternative?

Updated

7 months ago
Depends on: 1512047
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.