Closed
Bug 1503551
Opened 6 years ago
Closed 6 years ago
Make window.external.AddSearchProvider a dummy function
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
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)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
https://html.spec.whatwg.org/#external
"The AddSearchProvider() and IsSearchProviderInstalled() methods must do nothing."
Assignee | ||
Comment 1•6 years ago
|
||
Let's remove the support for this function just in nightly, as smaug suggested.
Attachment #9021500 -
Flags: review?(bugs)
Assignee | ||
Comment 2•6 years ago
|
||
The spec wants window.external
Attachment #9021502 -
Flags: review?(bugs)
Assignee | ||
Updated•6 years ago
|
Attachment #9021500 -
Attachment description: sidebar.patch → part 1 - dummy window.external methods
Comment 3•6 years ago
|
||
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•6 years ago
|
Attachment #9021502 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #9021500 -
Attachment is obsolete: true
Attachment #9021508 -
Flags: review?(bugs)
Assignee | ||
Comment 5•6 years ago
|
||
Just a changes in browser.ini which was brone. I guess I can keep the r+.
Assignee | ||
Updated•6 years ago
|
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
Updated•6 years ago
|
Updated•6 years ago
|
Keywords: site-compat
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/23850b9ba24d
https://hg.mozilla.org/mozilla-central/rev/7db2b627cacc
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Comment 8•6 years ago
|
||
Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/window-sidebar-and-window-external-addsearchprovider-have-been-deprecated/
Blocks: 1428302
Comment 9•6 years ago
|
||
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•6 years ago
|
||
Maybe we can have a webExtension to support this feature. Do you mind to file a bug?
Flags: needinfo?(evilpies)
Comment 11•6 years 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
Comment 12•6 years ago
|
||
(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)
Comment 13•6 years ago
|
||
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•6 years 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.
Updated•6 years ago
|
Keywords: dev-doc-needed
Comment 15•6 years ago
|
||
This also breaks installing search engines from https://mycroftproject.com/
Comment 16•6 years ago
|
||
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.
Comment 17•6 years ago
|
||
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)
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 18•6 years 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)
Comment 19•6 years ago
|
||
> 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!
Comment 20•6 years ago
|
||
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.
Comment 21•6 years ago
|
||
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•6 years ago
|
Component: DOM → DOM: Core & HTML
Comment hidden (advocacy) |
You need to log in
before you can comment on or make changes to this bug.
Description
•