Closed
Bug 691647
Opened 13 years ago
Closed 12 years ago
clean up nsISidebar (remove window.sidebar.addPanel/addPersistentPanel)
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: Gavin, Assigned: Gavin)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
I'd like to make the following changes to nsISidebar, as a precursor to bug 518929:
1) remove the add*Panel methods, since they are not standardized, rarely used, and not very well supported
2) fix the method signatures to avoid the use of string/wstring in favor of DOMString
1) is hopefully not a significant web compat hit (no other browsers implement these, AFAIK). It's a Netscape-era API, and while there are undoubtedly some legitimate users, we'd like to get rid of the front-end support for the sidebar feature entirely, and so this would be a first step towards that.
Given that the only implementations I know of are in JS, and that it is quite unlikely that there are any native code implementations, I think 2) shouldn't be a problem either.
Assignee | ||
Comment 1•13 years ago
|
||
I don't know who should review this, really...
Attachment #564443 -
Flags: superreview?(jonas)
Attachment #564443 -
Flags: review?(jst)
Assignee | ||
Comment 2•13 years ago
|
||
Assignee | ||
Comment 3•13 years ago
|
||
SeaMonkey also has an implementation, I'll file a separate bug on that.
http://mxr.mozilla.org/comm-central/source/suite/common/src/nsSidebar.js
Assignee | ||
Updated•13 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•13 years ago
|
Summary: clean up nsISidebar → clean up nsISidebar (remove window.sidebar.addPanel/addPersistentPanel)
Assignee | ||
Comment 4•13 years ago
|
||
Hmm, interesting that HTML 5 doesn't spec window.sidebar at all. We should probably consider getting rid of it entirely at some point - I think the only prominent user at this point might be http://mycroft.mozdev.org/ , and even they have OpenSearch and window.external support now :)
Comment 5•13 years ago
|
||
Comment on attachment 564443 [details] [diff] [review]
interface changes
While you're here, want to add a link to <http://www.whatwg.org/html/#the-external-interface>?
Comment 6•13 years ago
|
||
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #4)
> Hmm, interesting that HTML 5 doesn't spec window.sidebar at all. We should
> probably consider getting rid of it entirely at some point - I think the
> only prominent user at this point might be http://mycroft.mozdev.org/ , and
> even they have OpenSearch and window.external support now :)
Thanks for flagging.
Is there an alternative to window.sidebar(.addSearchEngine) for Sherlock?
It's a while since I looked properly but:
https://developer.mozilla.org/en/Adding_search_engines_from_web_pages
still says use window.sidebar.addSearchEngine
I vaguely remember discussing adding Sherlock engines with addSearchProvider but that'll make the capability detection more difficult.
Anyway, this isn't really relevant to this bug. Please CC me if there's a bug for complete removal of window.sidebar
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to Charles Caygill from comment #6)
> Anyway, this isn't really relevant to this bug. Please CC me if there's a
> bug for complete removal of window.sidebar
Will do!
Comment 8•13 years ago
|
||
Comment on attachment 564443 [details] [diff] [review]
interface changes
I'm all for cleaning this up and minimizing what we expose here, and I'd be in favor of removing this entirely too, but that would require some more investigation as to who would be affected by that change etc.
Attachment #564443 -
Flags: review?(jst) → review+
Assignee | ||
Updated•13 years ago
|
Attachment #564444 -
Flags: review?(dolske)
Comment 9•13 years ago
|
||
Comment on attachment 564444 [details] [diff] [review]
firefox implementation changes
Review of attachment 564444 [details] [diff] [review]:
-----------------------------------------------------------------
Kill it with fire. Laparoscopically.
Attachment #564444 -
Flags: review?(dolske) → review+
Attachment #564443 -
Flags: superreview?(jonas) → superreview+
Comment 10•13 years ago
|
||
Will there be *any* way left for websites to provide a sidebar addition to users?
So far, there still is the alternative with a <a> node, but if this gets removed, too:
https://bugzilla.mozilla.org/show_bug.cgi?id=692731#c11
then there is no way left to provide, for example, a newsticker or something like that to the user for open it in sidebar. To get this into sidebar, the user would have to open the link first, bookmark it and set checkbox "open in sidebar". Then click that link.
Assignee | ||
Comment 11•13 years ago
|
||
I'm planning to remove the sidebar from Firefox entirely eventually, so no, there won't be any way to trigger loading things there. But please don't turn this bug into a fight about sidebars, that debate can be had later :)
Comment 12•12 years ago
|
||
Gavin, did this ever land? If not, would now be a good time to land it?
Assignee | ||
Comment 13•12 years ago
|
||
I don't remember why this didn't land. I'd certainly welcome someone picking it up and unbitrotting the patches.
Comment 16•12 years ago
|
||
(In reply to Cykesiopka from comment #15)
> Created attachment 731390 [details] [diff] [review]
> Part 1: Interface Changes
>
> Unbitrotted.
(In reply to Cykesiopka from comment #16)
> Created attachment 731391 [details] [diff] [review]
> Part 2: Firefox Implementation Changes
>
> Unbitrotted.
Actually, do these need re-reviews?
Flags: needinfo?(gavin.sharp)
Assignee | ||
Comment 17•12 years ago
|
||
No, I don't think so. Thanks for reviving this bug!
Flags: needinfo?(gavin.sharp)
Keywords: checkin-needed
Assignee | ||
Comment 18•12 years ago
|
||
We will need to file followups to remove the dead code from the mobile/ and metro/ versions of this component, though.
Assignee | ||
Comment 19•12 years ago
|
||
Oh, we already have one for mobile (bug 692894, with a patch even). Just metro, then.
Assignee | ||
Comment 20•12 years ago
|
||
Comment 21•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3f2095fd4fc9
https://hg.mozilla.org/mozilla-central/rev/bcdc2f3743f9
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Keywords: site-compat
Comment 22•12 years ago
|
||
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/23
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23
https://developer.mozilla.org/en-US/docs/DOM/window.sidebar
Keywords: dev-doc-needed → dev-doc-complete
Comment 23•11 years ago
|
||
Thanks for breaking our web app (http://syntec.co.uk/index.php?p=ibagentandhomeworkers%20system) with this change: we use JavaScript to add a sidebar panel. Is there any alternative?
Comment 24•11 years ago
|
||
I'd like to second Seb A here as well. Removing the add*Panel methods is breaking our web app as well. Thanks alot.
Comment 25•11 years ago
|
||
It looks like a solution might be to use the new Social API (available to all web sites from Firefox 23.0 - the same release that removes Sidebars). However, the documentation is poor for newbies! I would be grateful if someone could provide content for the missing (red) links in https://developer.mozilla.org/en/docs/Social_API and finish https://developer.mozilla.org/en-US/docs/Social_API/Guide
Thanks!
Assignee | ||
Comment 26•11 years ago
|
||
Apologies for the breakage. Social API would indeed be a better alternative for this, we're working on trying to improve the documentation.
Comment 27•11 years ago
|
||
Added a mention of alternatives:
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23#DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•