437949 - Support for tel:

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

[Christian Sejersen]

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: support for tel uri

this adds a commandline handler that translates a tel or wtai uri into a voipto uri, and feeds that to the dbus.  The patch also contains the  bits to let the external protocol system know it exists.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 327650 [details] [diff] [review]
support for tel uri

as discussed on the #mobile channel, I'm going to look at other aproaches

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: implements new channel for tel uris

One concern I have is that all of the boiler plate code is from the doc that mfinkle pointed me to (http://www.nexgenmedia.net/docs/protocol/)

Comment 4

[Doug Turner (:dougt)]

Comment on attachment 328837 [details] [diff] [review]
implements new channel for tel uris

components/Makefile.in

wrong license stuff. clearly mfinkle didn't write this, and the (C) date is 2008.

Also remove the commented out lines in this file:

+
+#XPIDLSRCS = \
+#	$(NULL)
+
+#EXTRA_PP_COMPONENTS = \
+#	$(NULL)
+

The module should probably be "mobilecomps"

No XPIDL_MODULE is needed since you have not .idls.


components/teluri/Makefile.in

(c) dat is wrong.


components/teluri/nsTelProtocolHandler.js


Missing license.

"stolen liberally from www.nexgenmedia.net/docs/protocol" scares me.

Lies:

+// XPCOM constant definitions
+
+// Protocol definition
+    
+// ProtocolFactory definition
+    
+// TestModule definition
+        



use js xpcomUtils instead of all of your hand rolled "JS XPCOM component registration goop":

http://developer.mozilla.org/en/docs/How_to_Build_an_XPCOM_Component_in_Javascript#Using_XPCOMUtils


Spacing is al messed up in this file:

+  nsIProtocolHandler.URI_NOAUTH,
+  
+      allowPort: function(port, scheme)
+  {


+    var ios = Components.classes[kIOSERVICE_CONTRACTID].getService(nsIIOService);
+      
+        try{
+	//try voipto:
 etc.



Remove dump() statement


Save a baby, add a newline:

\ No newline at end of file



How does newChannel work?  we just ask the IOS to create a channel, and if it fails, we move on with different types (voipto, callto, wtai).  Where are those implemented? maybe in a different patch?

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: updated based on dougt's comments

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 328895 [details] [diff] [review]
updated based on dougt's comments

les.mk
>diff -r f6e767cbdcaf components/Makefile.in
>+
>+DIRS = teluri \
>+	$(NULL)

Can we call the folder "protocols" ? since we might have more than tel:

>+
>+include $(topsrcdir)/config/rules.mk
>+
>+MODULE = teluri

"protocols" here too

>diff -r f6e767cbdcaf components/teluri/nsTelProtocolHandler.js
>+
>+const kSCHEME = "tel";
>+const kPROTOCOL_NAME = "Tel Protocol";
>+const kPROTOCOL_CONTRACTID = "@mozilla.org/network/protocol;1?name=" + kSCHEME;
>+const kPROTOCOL_CID = Components.ID("d4bc06cc-fa9f-48ce-98e4-5326ca96ba28");
>+
>+// Mozilla defined
>+const kSIMPLEURI_CONTRACTID = "@mozilla.org/network/simple-uri;1";
>+const kIOSERVICE_CONTRACTID = "@mozilla.org/network/io-service;1";
>+const nsISupports = Components.interfaces.nsISupports;
>+const nsIIOService = Components.interfaces.nsIIOService;
>+const nsIProtocolHandler = Components.interfaces.nsIProtocolHandler;
>+const nsIURI = Components.interfaces.nsIURI;

If you don't like this block you can use the contract ID strings in the code itself and use

const Ci = Components.interfaces;

then Ci.nsIURI  in the code

>+
>+
>+/*        
>+function NSGetModule(){
>+  return TestModule;
>+}
>+
>+
>+
>+
>+/**
>+ * JS XPCOM component registration goop:
>+ *
>+ * We set ourselves up to observe the xpcom-startup category.  This provides
>+ * us with a starting point.
>+ */
>+/*
>+var TestModule = new Object();
>+
>+TestModule.registerSelf = function (compMgr, fileSpec, location, type)
>+{
>+  compMgr = compMgr.QueryInterface(Components.interfaces.nsIComponentRegistrar);
>+  compMgr.registerFactoryLocation(kPROTOCOL_CID,
>+				  kPROTOCOL_NAME,
>+				  kPROTOCOL_CONTRACTID,
>+				  fileSpec, 
>+				  location, 
>+				  type);
>+}
>+
>+  TestModule.getClassObject = function (compMgr, cid, iid)
>+{
>+  if (!cid.equals(kPROTOCOL_CID))
>+    throw Components.results.NS_ERROR_NO_INTERFACE;
>+  
>+  if (!iid.equals(Components.interfaces.nsIFactory))
>+         throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
>+  
>+  return ProtocolFactory;
>+}
>+    
>+    TestModule.canUnload = function (compMgr)
>+{
>+       return true;
>+}
>+      
>+      var ProtocolFactory = new Object();
>+
>+ProtocolFactory.createInstance = function (outer, iid)
>+{
>+  if (outer != null)
>+    throw Components.results.NS_ERROR_NO_AGGREGATION;
>+  
>+  if (!iid.equals(nsIProtocolHandler) && !iid.equals(nsISupports))
>+    throw Components.results.NS_ERROR_NO_INTERFACE;
>+  
>+  return new Protocol();
>+}
>+*/  

Remove this big commented block. XPCOMUtils handles this for you so we don't need to see it.

>+function TelProtocol()
>+{
>+}
>+
>+TelProtocol.prototype =
>+  {

fix indent on brace

>+
>+  classDescription: "Handler for tel URIs",
>+  classID:          Components.ID(kPROTOCOL_CID),
>+  contractID:       kPROTOCOL_CONTRACTID,
>+  QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIProtocolHandler]),
>+
>+
>+  scheme: kSCHEME,
>+  defaultPort: -1,
>+  protocolFlags: Components.interfaces.nsIProtocolHandler.URI_NORELATIVE | 
>+                 Components.interfaces.nsIProtocolHandler.URI_NOAUTH,

See all the Components.interfaces. ? They can disappear (or become Ci.)

>+  
>+    // strip away the kSCHEME: part
>+    phoneNumber = phoneNumber.substring(phoneNumber.indexOf(":") + 1, phoneNumber.length);    
>+    phoneNumber = encodeURI(phoneNumber);
>+    

Remove extra empty line

>+    
>+    var ios = Components.classes[kIOSERVICE_CONTRACTID].getService(nsIIOService);
>+    
>+    try{
>+	//try voipto:
>+      let channel = ios.newChannel("voipto:"+phoneNumber, null, null);
>+      return channel;
>+    }catch(e){
>+      //voipto not registered
>+    }
>+    try{
>+	//try callto:
>+      let channel = ios.newChannel("callto:"+phoneNumber, null, null);
>+      return channel;
>+    }catch(e){
>+      //callto not registered
>+    }

We normally put spaces between flow control statements and braces/parens ->
if (, not if(
try {, not try{

>+   
>+    //try wtai:  
>+    //this is our last equivalent protocol, so if it fails pass the exception on
>+    let channel = ios.newChannel("wtai:"+phoneNumber, null, null);
>+    return channel;
>+    

Remove empty line

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #4)
> (From update of attachment 328837 [details] [diff] [review])
> components/Makefile.in
> 
> wrong license stuff. clearly mfinkle didn't write this, and the (C) date is
> 2008.

Actually, this is mfinkle's code from another bug.  Its a race to see who lands the first mobile component. 

I could drop this hunk and be dependant on the other bug, but that seems like over kill.

> 
> Also remove the commented out lines in this file:
> 
> +
> +#XPIDLSRCS = \
> +#      $(NULL)
> +
> +#EXTRA_PP_COMPONENTS = \
> +#      $(NULL)
> +
>

I'm assuming mfinkle needs it for his component.
 
> The module should probably be "mobilecomps"

ehh.. these are components for a xul app, just like browser/xulrunner/mailnews has their own components

> 
> No XPIDL_MODULE is needed since you have not .idls.
> 
> 
> components/teluri/Makefile.in
> 
> (c) dat is wrong.

fixed

when did it turn 2008?

> 
> 
> components/teluri/nsTelProtocolHandler.js
> 
> 
> Missing license.
> 
> "stolen liberally from www.nexgenmedia.net/docs/protocol" scares me.

With the xpcom utils I didn't need any of the boiler plate code anymore.  Anything else that can be recognized from nexgenmedia.net can also be found in our unit tests (http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_protocolproxyservice.js#50)


> 
> Lies:
> 
> +// XPCOM constant definitions
> +
> +// Protocol definition
> +    
> +// ProtocolFactory definition
> +    
> +// TestModule definition
> +        

removed

> 
> 
> 
> use js xpcomUtils instead of all of your hand rolled "JS XPCOM component
> registration goop":
> 
> http://developer.mozilla.org/en/docs/How_to_Build_an_XPCOM_Component_in_Javascript#Using_XPCOMUtils
>

thank you
 
> 
> Spacing is al messed up in this file:
> 
> +  nsIProtocolHandler.URI_NOAUTH,
> +  
> +      allowPort: function(port, scheme)
> +  {
> 
> 
> +    var ios =
> Components.classes[kIOSERVICE_CONTRACTID].getService(nsIIOService);
> +      
> +        try{
> +       //try voipto:
>  etc.
> 

fixed, I think

> 
> 
> Remove dump() statement
> 

removed

> 
> Save a baby, add a newline:
> 
> \ No newline at end of file
> 
> 
> 
> How does newChannel work?  we just ask the IOS to create a channel, and if it
> fails, we move on with different types (voipto, callto, wtai).  Where are those
> implemented? maybe in a different patch?
> 

You've basically got it.  The goal is to see if the system can handle an equivalent scheme.  On the n800/n810 this is handled by the dbus handler (bug 441636), which is identified by the hildon action look up for a mime type (bug 437950).  On windows or mac this works with the skype or netmeeting registering to handle callto.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: updated based on mfinkle's comments

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: renamed folder from teluri to protocols

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: changed module name

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 328905 [details] [diff] [review]
changed module name

>diff -r f6e767cbdcaf components/protocols/nsTelProtocolHandler.js
>+  newChannel: function(aURI)

>+    // strip away the kSCHEME: part
>+    phoneNumber = phoneNumber.substring(phoneNumber.indexOf(":") + 1, phoneNumber.length); 

I would use const kSCHEME_REGEX = /\s*tel:\s*/, and then phoneNumber.replace(kSCHEME_REGEX, "");

Comment 12

[Stuart Parmenter]

brad landed this

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

on my n900, http://people.mozilla.org/~jmaher/tel.html works fine with the built in browser, but with Fennec this launches an application picker dialog!

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

what applications are listed in the application picker?

Comment 16

[Anna (Waverley)]

VERIFIED FIXED on:

Build ID: Mozilla/5.0 (Maemo; Linux armv7l; rv:6.0a1) Gecko/20110510
Firefox/6.0a1 Fennec/6.0a1

Device: Nokia N900 (Maemo GTK)

Comment 17

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Stuart Parmenter from comment #12)
> brad landed this

Happened to be digging around old code and found the push

https://hg.mozilla.org/mobile-browser/rev/9637869a28fa