Last Comment Bug 691647 - clean up nsISidebar (remove window.sidebar.addPanel/addPersistentPanel)
: clean up nsISidebar (remove window.sidebar.addPanel/addPersistentPanel)
Status: RESOLVED FIXED
: dev-doc-complete, site-compat
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla23
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 692896 284577 301424 518929 692894 859625
  Show dependency treegraph
 
Reported: 2011-10-03 20:02 PDT by :Gavin Sharp [email: gavin@gavinsharp.com]
Modified: 2013-08-12 06:32 PDT (History)
17 users (show)
gavin.sharp: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
interface changes (2.51 KB, patch)
2011-10-03 20:08 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
jst: review+
jonas: superreview+
Details | Diff | Splinter Review
firefox implementation changes (2.85 KB, patch)
2011-10-03 20:08 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
dolske: review+
Details | Diff | Splinter Review
Part 1: Interface Changes (2.34 KB, patch)
2013-03-29 17:27 PDT, :Cykesiopka
no flags Details | Diff | Splinter Review
Part 2: Firefox Implementation Changes (2.69 KB, patch)
2013-03-29 17:29 PDT, :Cykesiopka
no flags Details | Diff | Splinter Review

Description :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-03 20:02:46 PDT
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.
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-03 20:08:15 PDT
Created attachment 564443 [details] [diff] [review]
interface changes

I don't know who should review this, really...
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-03 20:08:42 PDT
Created attachment 564444 [details] [diff] [review]
firefox implementation changes
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-03 20:09:20 PDT
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
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-03 20:20:45 PDT
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 :Ms2ger (⌚ UTC+1/+2) 2011-10-04 03:01:55 PDT
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 Mycroft Project (CC) 2011-10-04 05:41:49 PDT
(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
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-04 09:23:06 PDT
(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 Johnny Stenback (:jst, jst@mozilla.com) 2011-10-05 21:44:30 PDT
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.
Comment 9 Justin Dolske [:Dolske] 2011-10-09 18:39:52 PDT
Comment on attachment 564444 [details] [diff] [review]
firefox implementation changes

Review of attachment 564444 [details] [diff] [review]:
-----------------------------------------------------------------

Kill it with fire. Laparoscopically.
Comment 11 Manuel Reimer 2011-10-20 08:52:47 PDT
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.
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-10-20 08:58:52 PDT
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 13 Johnny Stenback (:jst, jst@mozilla.com) 2012-07-16 19:30:49 PDT
Gavin, did this ever land? If not, would now be a good time to land it?
Comment 14 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-07-19 18:06:14 PDT
I don't remember why this didn't land. I'd certainly welcome someone picking it up and unbitrotting the patches.
Comment 15 :Cykesiopka 2013-03-29 17:27:36 PDT
Created attachment 731390 [details] [diff] [review]
Part 1: Interface Changes

Unbitrotted.
Comment 16 :Cykesiopka 2013-03-29 17:29:27 PDT
Created attachment 731391 [details] [diff] [review]
Part 2: Firefox Implementation Changes

Unbitrotted.
Comment 17 :Cykesiopka 2013-04-05 22:45:33 PDT
(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?
Comment 18 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-04-08 19:18:59 PDT
No, I don't think so. Thanks for reviving this bug!
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-04-08 19:20:04 PDT
We will need to file followups to remove the dead code from the mobile/ and metro/ versions of this component, though.
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-04-08 19:21:34 PDT
Oh, we already have one for mobile (bug 692894, with a patch even). Just metro, then.
Comment 24 Seb A 2013-08-09 06:35:02 PDT
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 25 Sandra Seilamann 2013-08-09 08:16:23 PDT
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 26 Seb A 2013-08-09 08:28:57 PDT
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!
Comment 27 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-08-11 21:42:12 PDT
Apologies for the breakage. Social API would indeed be a better alternative for this, we're working on trying to improve the documentation.
Comment 28 Kohei Yoshino [:kohei] 2013-08-12 06:32:53 PDT
Added a mention of alternatives:
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23#DOM

Note You need to log in before you can comment on or make changes to this bug.