Add Opera API for adding sidebars from websites

ASSIGNED
Assigned to

Status

ASSIGNED
7 years ago
7 years ago

People

(Reporter: Manuel.Spam, Assigned: Manuel.Spam)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 570060 [details] [diff] [review]
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.
(Assignee)

Updated

7 years ago
Summary: SeaMonkey should Opera API for adding sidebars from websites → Add Opera API for adding sidebars from websites
(Assignee)

Updated

7 years ago
Attachment #570060 - Flags: superreview?(mnyromyr)
Attachment #570060 - Flags: review?(mnyromyr)

Comment 1

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

Comment 2

7 years ago
(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

7 years ago
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.
Attachment #570060 - Flags: superreview?(neil)
Attachment #570060 - Flags: superreview?(mnyromyr)
Attachment #570060 - Flags: review?(mnyromyr)
Attachment #570060 - Flags: review+

Comment 4

7 years ago
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.

Updated

7 years ago
Duplicate of this bug: 192030
You need to log in before you can comment on or make changes to this bug.