372441 - Implement registerProtocolHandler for arbitrary protocols

Status: VERIFIED FIXED

(Firefox :: File Handling, enhancement, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

It should be possible for web applications to act as handlers for content types and protocols, as specified in the WHATWG HTML5 draft.

Comment 1

[Asaf Romano (gone)]

Let me know if you need help wrt. the current implementation.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Mano: thanks!  I bet I'll have questions for you...

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: web protocol handler strawman impl, v1

Here's an unfinished strawman web-based protocol handler patch with lots of hardcoded junk just as a proof of concept based on a suggestion by biesi.  Currently it only detects mailto URLs, and then does evil url munging to send it to Yahoo mail, because that was a good a URL to test with as any.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: web protocol handler strawman impl, v2

Doesn't call OnChannelRedirect from inside AsyncOpen, as suggested by biesi.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

I've spun the protocol handler work off to bug 380415 so that it can be tracked independently from this entire PRD item.

Comment 7

[R Pruitt]

Not sure this is the right place but since the title is arbitrary protocols,  chrome:// doesn't work for me:

  const nfUri = "chrome://newsfox/content/addurl.xul?%s";
  // following 'appropriate' method doesn't handle chrome://
  navigator.registerContentHandler("application/vnd.mozilla.maybe.feed", nfUri, "Newsfox");

does not seem to register (I'm following http://developer.mozilla.org/en/docs/DOM:window.navigator.registerContentHandler).  It gives no errors, but the first time about:config is accessed, gives information errors:

 No chrome package registered for chrome://navigator/locale/navigator.properties
 No chrome package registered for chrome://navigator-region/locale/region.properties
 No chrome package registered for chrome://communicator-region/locale/region.properties

Looking for an open spot and setting

user_pref("browser.contentHandlers.types.3.title", "Newsfox");
user_pref("browser.contentHandlers.types.3.uri", "chrome://newsfox/content/addurl.xul?%s");

via Javascript does seem to work though, although not reliably in my experience. If it's just a security issue, why does it let me do the second?

Newsfox and presumably WizzRSS and others? would use this.

Comment 8

[Mike Connor [:mconnor]]

Missing the freeze, moving out.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Since content-handling is not going to make Firefox3, it's been spun off into bug 391286.

Comment 10

[Shawn Wilsher :sdwilsh]

Attachment: Already added notification bar

Comment 11

[Shawn Wilsher :sdwilsh]

Attachment: ask to register

Comment 12

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

still unclear as to who should review this...

This should work, in theory, once all the bugs blocking this are fixed.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 275721 [details]
Already added notification bar

ui-r+ with the following caveats:

- please file a bug asking for an icon for this notification message, request blocking firefox 3

- drop the quotes around the name of the application

- change the string to "%appName has already been added as an application for %protocolType links" where %protocolType is a nice english mapping (if we have that) or just the protocol type (if we don't). So in this case the string would be: "Ical Share has already been added as an application for webcal links."

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 275722 [details]
ask to register

ui-r+ with the following caveats:

- please file a bug asking for an icon for this notification message, request blocking firefox 3

- drop the quotes around the name of the application

- change the string to "Add %appName (%appdomain) as an application for %protocolType links?" where %protocolType is a nice english mapping (if we have that) or just the protocol type (if we don't). So in this case the string would be: "Add Ical Share (icalshare.com) as an application for webcal links?"

- make the button "Add Application"

Comment 15

[Asaf Romano (gone)]

Comment on attachment 275726 [details] [diff] [review]
v1.0

>? browser/locales/en-US/chrome/browser/feeds/.subscribe.properties.swp
>Index: browser/components/feeds/src/WebContentConverter.js
>===================================================================

>   /**
>+   * Determines if a web handler is already registered.
>+   *
>+   * @param aProtocol
>+   *        The scheme of the web handler we are checking for.
>+   * @param aURITemplate
>+   *        The URI template that the handler uses to handle the protocol.
>+   * @return true if it is already registered, false otherwise.
>+   */
>+  _protocolHandlerRegistered: function(aProtocol, aURITemplate) {

nit: name it.

>+    var eps = Cc["@mozilla.org/uriloader/external-protocol-service;1"].
>+                  getService(Ci.nsIExternalProtocolService);

[general] nit: getService should be aligned with Cc.


>+    var handler = ios.getProtocolHandler(aProtocol)
>+                     .QueryInterface(Ci.nsIExternalProtocolHandler);
>+    if (!handler) {

If getProtocolHandler returned null, this would throw before you get to the (!handler) check.

>+      // This is handled internally, so we don't want them to register
>+      // XXX this should be a "security exception" according to spec, but that
>+      // isn't defined yet.
>+      throw("Permission denied to add " + aURIString + "as a protocol handler");
>+    }


>+    var buttons, message;
>+    if (this._protocolHandlerRegistered(aProtocol, uri.spec)) {
>+      message = this._getFormattedString("protocolHandlerRegistered", [aTitle]);
>+    } else {

nit: remove the braces.



>+      // Now Ask the user and provide the proper callback
>+      message = this._getFormattedString("addProtocolHandler", [aTitle, uri.host]);
>+      var notificationValue = "Protocol Registration:  " + aProtocol;
>+      var notificationIcon = uri.prePath + "/favicon.ico";

Could we use the favicon service here (in the form of moz-anno:// uri)?

>+      var self = this;
>+      var addButton = {
>+        _outer: self,

unused.

>+        label: self._getString("addProtocolHandlerAddButton"),
>+        accessKey: self._getString("addHandlerAddButtonAccesskey"),
>+        protocolInfo: { protocol: aProtocol, uri: uri.spec, name: aTitle },
>+
>+        callback:
>+        function WCCR_addProtocolHandlerButtonCallback(aNotification, aButtonInfo) {


>+          // avoid reference cycles
>+          aButtonInfo._outer = null;

see above.

Comment 16

[Robert Sayre]

Comment on attachment 275726 [details] [diff] [review]
v1.0

dogpiling here just so I write down what I mentioned to sdwilsh in irc.

>? browser/locales/en-US/chrome/browser/feeds/.subscribe.properties.swp
>Index: browser/components/feeds/src/WebContentConverter.js
>===================================================================
>+    for (let i = 0; i < handlers.length; i++) {
>+      try { // We only want to test web handlers
>+        let handler = handlers.queryElementAt(i, Ci.nsIWebHandlerApp);
>+      } catch (e) { /* it wasn't a web handler */ }
>+      if (handler.uriTemplate == aURITemplate) {
>+        handlers.removeElementAt(i);
>+        return;
>+      }
>     }

buggy let scope.

>+    if (!handler) {
>+      // This is handled internally, so we don't want them to register
>+      // XXX this should be a "security exception" according to spec, but that
>+      // isn't defined yet.
>+      throw("Permission denied to add " + aURIString + "as a protocol handler");
>+    }
>+    
>+    try {
>+      var uri = this._makeURI(aURIString);
>+    } catch (ex) {
>+      // not supposed to throw according to spec
>+      return; 
>+    }
>+    
>+    // If the uri doesn't contain '%s', it won't be a good protocol handler
>+    if (uri.spec.indexOf("%s") < 0)
>+      throw NS_ERROR_DOM_SYNTAX_ERR; 

Try share this setup code?

Comment 17

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

(In reply to comment #13)
> - please file a bug asking for an icon for this notification message, request
> blocking firefox 3
Bug 391576

> - drop the quotes around the name of the application
done

> - change the string to "%appName has already been added as an application for
> %protocolType links" where %protocolType is a nice english mapping (if we have
> that) or just the protocol type (if we don't). So in this case the string would
> be: "Ical Share has already been added as an application for webcal links."
done with localization note so that the string makes sense when they try to translate it.

(In reply to comment #14)
> - change the string to "Add %appName (%appdomain) as an application for
> %protocolType links?" where %protocolType is a nice english mapping (if we have
> that) or just the protocol type (if we don't). So in this case the string would
> be: "Add Ical Share (icalshare.com) as an application for webcal links?"
> 
> - make the button "Add Application"
done with localization note so that the string makes sense when they try to translate it.

also addresses review comments.

Comment 18

[Asaf Romano (gone)]

Comment on attachment 276028 [details] [diff] [review]
v1.1

>Index: browser/components/feeds/src/WebContentConverter.js
>===================================================================

>+  function WCCR_removeProtocolHandler(aProtocol, aURITemplate) {
>+    var eps = Cc["@mozilla.org/uriloader/external-protocol-service;1"].
>+              getService(Ci.nsIExternalProtocolService);
>+    var handlerInfo = eps.getProtocolHandlerInfo(aProtocol);
>+    var handlers =  handlerInfo.possibleApplicationHandlers;
>+    for (let i = 0; i < handlers.length; i++) {
>+      try { // We only want to test web handlers
>+        let handler = handlers.queryElementAt(i, Ci.nsIWebHandlerApp);
>+        if (handler.uriTemplate == aURITemplate) {
>+          handlers.removeElementAt(i);
>+          var hs = Cc["@mozilla.org/uriloader/handler-service;1"].
>+                   getService(Ci.nsIHandlerService);
>+          hs.store(handlerInfo);
>+          return;
>+        }
>+      } catch (e) { /* it wasn't a web handler */ }

shouldn't the try block only wrap queryElementAt?

>+  _checkAndGetURI:
>+  function WCCR_checkAndGetURI(aURIString)
>+  {
>+    try {
>+      var uri = this._makeURI(aURIString);
>+    } catch (ex) {
>+      // not supposed to throw according to spec
>+      return; 
>+    }
>+    

trailing spaces.

>   /**
>    * See nsIWebContentHandlerRegistrar
>    */
>   registerProtocolHandler: 
>-  function WCCR_registerProtocolHandler(aProtocol, aURI, aTitle, aContentWindow) {
>-    // not yet implemented
>-    return;
>+  function WCCR_registerProtocolHandler(aProtocol, aURIString, aTitle, aContentWindow) {
>+    LOG("registerProtocolHandler(" + aProtocol + "," + aURIString + "," + aTitle + ")");
>+    
>+    // First, check to make sure this isn't already handled internally (we don't
>+    // want to let them take over, say "chrome").
>+    var ios = Cc["@mozilla.org/network/io-service;1"].
>+              getService(Ci.nsIIOService);
>+    try {
>+      ios.getProtocolHandler(aProtocol)
>+         .QueryInterface(Ci.nsIExternalProtocolHandler);

which one would throw here, getProtocolHandler or QueryInterface? should both result in a "permission denied" exception? 

>+    } catch (e) {
>+      // This is handled internally, so we don't want them to register
>+      // XXX this should be a "security exception" according to spec, but that
>+      // isn't defined yet.
>+      throw("Permission denied to add " + aURIString + "as a protocol handler");
>+    }
>+    

trailing spaces.

>+    else {
>+      // Now Ask the user and provide the proper callback
>+      message = this._getFormattedString("addProtocolHandler",
>+                                         [aTitle, uri.host, aProtocol]);
>+      var fis = Cc["@mozilla.org/browser/favicon-service;1"].
>+                getService(Ci.nsIFaviconService);
>+      var notificationIcon = fis.getFaviconLinkForPage(uri).spec;
>+      dump("icon uri = " + notificationIcon + "\n");

remove this before checkin please.

>+      var self = this;
>+      var addButton = {
>+        label: self._getString("addProtocolHandlerAddButton"),
>+        accessKey: self._getString("addHandlerAddButtonAccesskey"),

|self| is not necessary, in this context |this| points to the service.

r=mano otherwise.

Comment 19

[Shawn Wilsher :sdwilsh]

(In reply to comment #18)
> shouldn't the try block only wrap queryElementAt?
Except that if it throws, we shouldn't do any of that other stuff.

> which one would throw here, getProtocolHandler or QueryInterface? should both
> result in a "permission denied" exception? 
I can use instanceof instead, removed try-catch

Comment 20

[Shawn Wilsher :sdwilsh]

Attachment: v1.2

for checkin

Note, that this will not show anything in the UI until Bug 390890 lands.

Comment 21

[Shawn Wilsher :sdwilsh]

Again, this doesn't do anything with the UI.  That is Bug 390890!

Checking in browser/components/feeds/src/WebContentConverter.js;
new revision: 1.21; previous revision: 1.20
Checking in browser/locales/en-US/chrome/browser/feeds/subscribe.properties;
new revision: 1.6; previous revision: 1.5

Comment 22

[Sergey Sokoloff]

(In reply to comment #15)

> >+      // Now Ask the user and provide the proper callback
> >+      message = this._getFormattedString("addProtocolHandler", [aTitle, uri.host]);
> >+      var notificationValue = "Protocol Registration:  " + aProtocol;
> >+      var notificationIcon = uri.prePath + "/favicon.ico";
> 
> Could we use the favicon service here (in the form of moz-anno:// uri)?

Just to make it clear for myself next time I visit this bug:

1) The favicon service is documented at http://developer.mozilla.org/en/docs/Places:Favicon_Service

2) The use of moz-anno:// URL scheme is slightly documented at http://developer.mozilla.org/en/docs/Places:Annotation_Service -- but does not have any obvious examples yet, I mean there's no examples in moz-anno://...... form.

Comment 23

[Marcia Knous [:marcia]]

Litmus Triage Team: Adding ctalbert to the bug for Litmus coverage.

Comment 24

[Eric Shepherd [:sheppy]]

This has now been documented:

http://developer.mozilla.org/en/docs/DOM:window.navigator.registerProtocolHandler

Comment 25

[Dan Mosedale (:dmosedale, :dmose)]

The doc has an issue: it's not arbitrary text that's insert in place of the %s, it's the escaped URI of the document to be handled.

I think we also want a bit of documentation about the end-to-end experience of making an existing web-app use this API (talking about both the registerProtocolHandler call, as well as the handling of the URIs in question).
That might be a different document, though.

Comment 26

[Eric Shepherd [:sheppy]]

Is there anyone that has already done some work with making web apps support being a protocol handler that could write up a little article (or at least some notes; I can polish it into a real article) on how to do it?

I can do it myself, but I like to ask for contributions from folks that already know how to do it -- saves time and is more likely to more quickly produce an accurate article.

I've fixed the %s issue in the doc; thanks.

Comment 27

[Dan Mosedale (:dmosedale, :dmose)]

sheppy: one more thing that would be good to tweak in the MDC page would be to just use some generic domain in the example rather than gmail: at some point we hope to constrain handlers to only register stuff on their own host.

Comment 28

[Eric Shepherd [:sheppy]]

I've updated the doc to both mention that the protocol must be registered from the same domain as the protocol handler, and I've also replaced the examples with a generic example.

Still needs some work, and could use a step-by-step guide on how to do this from end-to-end, but it's better.

Comment 29

[Eric Shepherd [:sheppy]]

I've filed bug 420876 to cover the needed tutorial, and am marking this issue as complete.

Comment 30

[cmtalbert]

This is pretty well covered in Litmus:
https://litmus.mozilla.org/show_test.cgi?searchType=by_category&product_id=1&branch_id=15&testgroup_id=56&subgroup_id=820

And
https://litmus.mozilla.org/show_test.cgi?searchType=by_category&product_id=1&branch_id=15&testgroup_id=55&subgroup_id=884

--> Mark this as verified too.  This support has landed.