384139 - move openURL() to a reusable place in toolkit

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Robert Kaiser]

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

[Philip Chee]

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

Comment 2

[Philip Chee]

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

[Philip Chee]

> I think this bug should have been filed under Toolkit not Core.
Core -> Toolkit

Comment 4

[Robert Kaiser]

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.

Comment 5

[Robert Kaiser]

Attachment: 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!

Comment 6

[Philip Chee]

(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

Comment 7

[Robert Kaiser]

Attachment: 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.

Comment 8

[Robert Kaiser]

Checked in. I'll leave it to others to add further consumers as mentioned in comment #2 ;-)