move openURL() to a reusable place in toolkit

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: kairo, Assigned: kairo)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

12 years ago
The openURL() function used by EM to open "get new themes" etc. should probably move to a globally accessible place in toolkit so that it can be reused in other places where we need to open a URL but don't know if we're a browser ourself.
We have such cases in all sorts of shared code in toolkit and also shared mailnews, for example.

Having once function to use at several places eases life for all those consumers.

Comment 1

12 years ago
I suggest globalOverlay.js since extensions.xul already imports this script.

Comment 2

12 years ago
qv Bug 349309 Comment 9 :
------------------------------------------------------------
Steffen Wilberg   2007-05-30 08:31:22 PDT

That's a lot of generic-looking code, yet it's in extensions.js.
It would be nice to have that in a generic location, so that it can be used by
other toolkit components as well (bug 345001, bug 263433, bug 262808)?
Also see visitLink() in globalOverlay.js, which is for the about dialog:
http://mxr.mozilla.org/seamonkey/source/toolkit/content/globalOverlay.js#150
------------------------------------------------------------
Right visitLink() would certainly benefit from my patch to openUrl()

Kairo: I think this bug should have been filed under Toolkit not Core.

Comment 3

11 years ago
> I think this bug should have been filed under Toolkit not Core.
Core -> Toolkit
Component: General → XUL Widgets
Product: Core → Toolkit
QA Contact: general → xul.widgets
(Assignee)

Comment 4

11 years ago
From a look today, I think openURL() should probably go into contentAreaUtils.js - we could do a patch here that moves it from extensions.js here and makes EM use that version, other consumers of similar functions (like openUILink() in Firefox' utilityOverlay.js) can be ported over to this function later.
(Assignee)

Comment 5

11 years ago
Created attachment 275817 [details] [diff] [review]
move openURL() from extensions to contentAreaUtils

Uh, I wouldn't have believed this to be so easy... Still need to test other apps, but from what I have seen so far, this actually works fine!
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #275817 - Flags: review?(benjamin)

Comment 6

11 years ago
(In reply to comment #5)
> Uh, I wouldn't have believed this to be so easy...
That's because it isn't.

+  var ios = Components.classes["@mozilla.org/network/io-service;1"]
+                            .getService(nsIIOService);
--> Components.interfaces.nsIIOService

+    var loadgroup = Components.classes["@mozilla.org/network/load-group;1"]
+                              .createInstance(nsILoadGroup);
--> Components.interfaces.nsILoadGroup

+    var appstartup = Components.classes["@mozilla.org/toolkit/app-startup;1"]
+                               .getService(nsIAppStartup);
--> Components.interfaces.nsIAppStartup

+        if (iid.equals(nsILoadGroup))
--> Components.interfaces.nsILoadGroup

You can't rely on these constants being available from extensions.js
(Assignee)

Comment 7

11 years ago
Created attachment 275962 [details] [diff] [review]
move openURL() from extensions to contentAreaUtils, v1.1

Right, the shorthands aren't defined everywhere. nsILoadGroup would have been defined locally, but it looks more readable to me to remove that as well when we can not use the other shorthands.
Attachment #275817 - Attachment is obsolete: true
Attachment #275962 - Flags: review?(benjamin)
Attachment #275817 - Flags: review?(benjamin)

Updated

11 years ago
Attachment #275962 - Flags: review?(benjamin) → review+
(Assignee)

Comment 8

11 years ago
Checked in. I'll leave it to others to add further consumers as mentioned in comment #2 ;-)
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Duplicate of this bug: 345001

Updated

11 years ago
Blocks: 263433

Updated

11 years ago
Blocks: 262808
You need to log in before you can comment on or make changes to this bug.