Closed
Bug 372441
Opened 18 years ago
Closed 17 years ago
Implement registerProtocolHandler for arbitrary protocols
Categories
(Firefox :: File Handling, enhancement, P1)
Firefox
File Handling
Tracking
()
VERIFIED
FIXED
Firefox 3 alpha8
People
(Reporter: dmosedale, Assigned: sdwilsh)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete, Whiteboard: CON-001a)
Attachments
(3 files, 4 obsolete files)
(deleted),
image/png
|
beltzner
:
ui-review+
|
Details |
(deleted),
image/png
|
beltzner
:
ui-review+
|
Details |
(deleted),
patch
|
Details | Diff | Splinter Review |
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•18 years ago
|
||
Let me know if you need help wrt. the current implementation.
Reporter | ||
Comment 2•18 years ago
|
||
Mano: thanks! I bet I'll have questions for you...
Reporter | ||
Comment 3•17 years ago
|
||
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•17 years ago
|
||
Doesn't call OnChannelRedirect from inside AsyncOpen, as suggested by biesi.
Attachment #264405 -
Attachment is obsolete: true
Reporter | ||
Comment 5•17 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•17 years ago
|
Attachment #264421 -
Attachment is obsolete: true
Updated•17 years ago
|
Flags: blocking-firefox3?
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Whiteboard: CON-001a
Updated•17 years ago
|
Target Milestone: --- → Firefox 3 alpha6
Reporter | ||
Updated•17 years ago
|
Keywords: dev-doc-needed
Reporter | ||
Updated•17 years ago
|
Target Milestone: Firefox 3 alpha6 → Firefox 3 beta1
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•17 years ago
|
||
Missing the freeze, moving out.
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Assignee | ||
Updated•17 years ago
|
Assignee: dmose → sdwilsh
Priority: -- → P1
Version: unspecified → Trunk
Reporter | ||
Comment 9•17 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•17 years ago
|
Summary: Implement registerProtocolHandler for arbitrary content/protocols → Implement registerProtocolHandler for arbitrary protocols
Assignee | ||
Updated•17 years ago
|
Assignee | ||
Comment 10•17 years ago
|
||
Attachment #275721 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 11•17 years ago
|
||
Attachment #275722 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 12•17 years ago
|
||
still unclear as to who should review this...
This should work, in theory, once all the bugs blocking this are fixed.
Assignee | ||
Updated•17 years ago
|
Attachment #275726 -
Flags: review?(sayrer)
Reporter | ||
Updated•17 years ago
|
Whiteboard: CON-001a → CON-001a [needs ui-r(beltzner), r(sayrer)]
Comment 13•17 years ago
|
||
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 14•17 years ago
|
||
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 15•17 years ago
|
||
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•17 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 | ||
Comment 17•17 years ago
|
||
(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•17 years ago
|
Attachment #276028 -
Attachment is patch: true
Attachment #276028 -
Attachment mime type: application/octet-stream → text/plain
Comment 18•17 years ago
|
||
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•17 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•17 years ago
|
Whiteboard: CON-001a [needs ui-r(beltzner), r(sayrer)] → CON-001a
Assignee | ||
Comment 20•17 years ago
|
||
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•17 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
Closed: 17 years ago
Flags: in-testsuite?
Flags: in-litmus?
Resolution: --- → FIXED
Comment 22•17 years ago
|
||
(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•17 years ago
|
||
Litmus Triage Team: Adding ctalbert to the bug for Litmus coverage.
Comment 24•17 years ago
|
||
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•17 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
Comment 26•17 years ago
|
||
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•17 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.
Comment 28•17 years ago
|
||
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•17 years ago
|
||
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•16 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.
Description
•