Implement registerProtocolHandler for arbitrary protocols

VERIFIED FIXED in Firefox 3 alpha8

Status

()

Firefox
File Handling
P1
enhancement
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: dmose, Assigned: sdwilsh)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
Firefox 3 alpha8
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +
in-testsuite ?
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: CON-001a, URL)

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

11 years ago
It should be possible for web applications to act as handlers for content types and protocols, as specified in the WHATWG HTML5 draft.
Let me know if you need help wrt. the current implementation.
(Reporter)

Comment 2

11 years ago
Mano: thanks!  I bet I'll have questions for you...
(Reporter)

Updated

11 years ago
Blocks: 372949
(Reporter)

Comment 3

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

Comment 4

10 years ago
Created attachment 264421 [details] [diff] [review]
web protocol handler strawman impl, v2

Doesn't call OnChannelRedirect from inside AsyncOpen, as suggested by biesi.
Attachment #264405 - Attachment is obsolete: true
(Reporter)

Updated

10 years ago
Depends on: 380415
(Reporter)

Comment 5

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

Updated

10 years ago
Attachment #264421 - Attachment is obsolete: true

Updated

10 years ago
Flags: blocking-firefox3?
Flags: blocking-firefox3? → blocking-firefox3+
Whiteboard: CON-001a

Updated

10 years ago
Target Milestone: --- → Firefox 3 alpha6
(Reporter)

Updated

10 years ago
Depends on: 385065
(Reporter)

Updated

10 years ago
No longer depends on: 385065
(Reporter)

Updated

10 years ago
Depends on: 385101
(Reporter)

Updated

10 years ago
Depends on: 385104
(Reporter)

Updated

10 years ago
Depends on: 385106
(Reporter)

Updated

10 years ago
Depends on: 385114
(Reporter)

Updated

10 years ago
No longer depends on: 385104
(Reporter)

Updated

10 years ago
Depends on: 377784
(Reporter)

Updated

10 years ago
Keywords: dev-doc-needed
(Reporter)

Updated

10 years ago
Duplicate of this bug: 381498
(Reporter)

Updated

10 years ago
Target Milestone: Firefox 3 alpha6 → Firefox 3 beta1

Comment 7

10 years ago
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.
Missing the freeze, moving out.
Target Milestone: Firefox 3 M7 → Firefox 3 M8
(Assignee)

Updated

10 years ago
Assignee: dmose → sdwilsh
Priority: -- → P1
Version: unspecified → Trunk
(Assignee)

Updated

10 years ago
Depends on: 391150
(Assignee)

Updated

10 years ago
Depends on: 385740
(Assignee)

Updated

10 years ago
Depends on: 391279
(Reporter)

Comment 9

10 years ago
Since content-handling is not going to make Firefox3, it's been spun off into bug 391286.
Summary: Implement registerProtocolHandler and registerContentHandler for arbitrary content/protocols → Implement registerProtocolHandler for arbitrary content/protocols
(Reporter)

Updated

10 years ago
Summary: Implement registerProtocolHandler for arbitrary content/protocols → Implement registerProtocolHandler for arbitrary protocols
(Assignee)

Updated

10 years ago
Blocks: 380415
No longer depends on: 380415
(Assignee)

Updated

10 years ago
No longer depends on: 385101
(Assignee)

Updated

10 years ago
No longer depends on: 385106
(Assignee)

Updated

10 years ago
No longer depends on: 385114
(Assignee)

Updated

10 years ago
No longer blocks: 372949
(Assignee)

Comment 10

10 years ago
Created attachment 275721 [details]
Already added notification bar
Attachment #275721 - Flags: ui-review?(beltzner)
(Assignee)

Comment 11

10 years ago
Created attachment 275722 [details]
ask to register
Attachment #275722 - Flags: ui-review?(beltzner)
(Assignee)

Comment 12

10 years ago
Created attachment 275726 [details] [diff] [review]
v1.0

still unclear as to who should review this...

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

Updated

10 years ago
Attachment #275726 - Flags: review?(sayrer)
(Reporter)

Updated

10 years ago
Whiteboard: CON-001a → CON-001a [needs ui-r(beltzner), r(sayrer)]
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."
Attachment #275721 - Flags: ui-review?(beltzner) → ui-review+
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"
Attachment #275722 - Flags: ui-review?(beltzner) → ui-review+
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.
Attachment #275726 - Flags: review?(sayrer) → review-

Comment 16

10 years ago
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?
Attachment #275726 - Flags: review-
(Assignee)

Updated

10 years ago
Blocks: 391576
(Assignee)

Comment 17

10 years ago
Created attachment 276028 [details] [diff] [review]
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.
Attachment #275726 - Attachment is obsolete: true
Attachment #276028 - Flags: review?(mano)
(Reporter)

Updated

10 years ago
Attachment #276028 - Attachment is patch: true
Attachment #276028 - Attachment mime type: application/octet-stream → text/plain
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.
Attachment #276028 - Flags: review?(mano) → review+
(Assignee)

Comment 19

10 years ago
(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
(Assignee)

Updated

10 years ago
Whiteboard: CON-001a [needs ui-r(beltzner), r(sayrer)] → CON-001a
(Assignee)

Comment 20

10 years ago
Created attachment 276713 [details] [diff] [review]
v1.2

for checkin

Note, that this will not show anything in the UI until Bug 390890 lands.
Attachment #276028 - Attachment is obsolete: true
(Assignee)

Comment 21

10 years ago
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
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite?
Flags: in-litmus?
Resolution: --- → FIXED
(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.
Litmus Triage Team: Adding ctalbert to the bug for Litmus coverage.
This has now been documented:

http://developer.mozilla.org/en/docs/DOM:window.navigator.registerProtocolHandler
Keywords: dev-doc-needed → dev-doc-complete
(Reporter)

Comment 25

10 years ago
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.
Keywords: dev-doc-complete → dev-doc-needed
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.
(Reporter)

Comment 27

10 years ago
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.
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.
I've filed bug 420876 to cover the needed tutorial, and am marking this issue as complete.
Keywords: dev-doc-needed → dev-doc-complete

Comment 30

9 years ago
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.
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.