697799 - Add Opera API for adding sidebars from websites

Status: ASSIGNED

(SeaMonkey :: Sidebar, defect)

Description

[Manuel Reimer]

Attachment: First patch

To allow importing a sidebar into opera, a webmaster can use a link like this:

<a href="http://domain.tld/sidebar.html" rel="sidebar" title="sidebar-title">...

If we support this in SeaMonkey, nearly all panels, made for opera, should work with SeaMonkey, too. Probably this could even be a replacement for the existing API which gets less relevant as Firefox dropped it.

Comment 1

[Philip Chee]

Do you have a page with an Opera sidebar we can test this with?

Comment 2

[Karsten Düsterloh]

(In reply to Philip Chee from comment #1)
> Do you have a page with an Opera sidebar we can test this with?

Opera sidebar directory:
http://my.opera.com/community/customize/panel/directory/

Comment 3

[Karsten Düsterloh]

Comment on attachment 570060 [details] [diff] [review]
First patch

>+function SidebarAddPanel(aTitle, aContentURL)

It'd probably better to call it "SidebarAddLinkRelPanel" or "SidebarAddWebPanel".

>+{
>+  // Check if URL is allowed in sidebar
>+  if (!/(^https?:|^ftp:)/i.test(aContentURL))
>+    throw "Script attempted to add sidebar panel from illegal source";

Why are random *URLs* more secure than others? Why should it be more secure to not allow file: URLs?
(Apart from Windows bugs.)
It'd make more sense to run it past a scam test, etc. 
After all, we don't allow installation without confirmation anyway.

Comment 4

[[e-mail]]

Comment on attachment 570060 [details] [diff] [review]
First patch

Sorry for the delay in the review, I've been busier than usual recently. The patch works well but I would appreciate a few tweaks as follows:

>+        if (component == "navigator:browser" && event.button == 0 &&
Should we check for modifiers as well?

>+            ceParams.linkNode.getAttribute("rel") == "sidebar" &&
>+            ceParams.linkNode.getAttribute("title")) {
Use ceParams.linkNode.rel == "sidebar" && ceParams.linkNode.title
(and title again on the next line)

>+      stringBundle = Services.strings.createBundle("chrome://communicator/locale/sidebar/sidebar.properties");
>+      brandStringBundle = Services.strings.createBundle("chrome://branding/locale/brand.properties");
These should use <stringbundle> elements in the XUL (navigator.js defines gBrandBundle for brand.properties but you would have to create your own for sidebar.properties) and you probably don't need the try/catch any more.

>+        dialogMessage = dialogMessage.replace(/%url%/, aContentURL);
>+        dialogMessage = dialogMessage.replace(/%name%/, sidebarName);
[We should really use getFormattedString for this, but that's a separate bug.]

>+    Services.prompt.alert(null, titleMessage, dialogMessage);
Use window for all prompts, since we actually have one now.