Closed Bug 1503551 Opened 3 years ago Closed 3 years ago

Make window.external.AddSearchProvider a dummy function

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: baku, Assigned: baku)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(2 files, 1 obsolete file)

https://html.spec.whatwg.org/#external
"The AddSearchProvider() and IsSearchProviderInstalled() methods must do nothing."
Let's remove the support for this function just in nightly, as smaug suggested.
Attachment #9021500 - Flags: review?(bugs)
The spec wants window.external
Attachment #9021502 - Flags: review?(bugs)
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+
Attachment #9021502 - Flags: review?(bugs) → review+
Attachment #9021500 - Attachment is obsolete: true
Attachment #9021508 - Flags: review?(bugs)
Just a changes in browser.ini which was brone. I guess I can keep the r+.
Attachment #9021508 - Flags: review?(bugs)
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
Keywords: site-compat
https://hg.mozilla.org/mozilla-central/rev/23850b9ba24d
https://hg.mozilla.org/mozilla-central/rev/7db2b627cacc
Status: NEW → RESOLVED
Closed: 3 years 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.
Maybe we can have a webExtension to support this feature. Do you mind to file a bug?
Flags: needinfo?(evilpies)
Blocks: 1346123
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.
(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.
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)
> 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.
Blocks: 1509061
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?
Depends on: 1512047
No longer depends on: 1504338
Component: DOM → DOM: Core & HTML
Blocks: 615761
Blocks: 1640138
No longer blocks: 1640138
You need to log in before you can comment on or make changes to this bug.