Last Comment Bug 645729 - Make sure 'body' data is sent along with special links (mailto and sms)
: Make sure 'body' data is sent along with special links (mailto and sms)
Status: VERIFIED FIXED
:
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: All All
: P4 normal (vote)
: Firefox 7
Assigned To: Alex Pakhotin (:alexp)
:
:
Mentors:
Depends on:
Blocks: 647338
  Show dependency treegraph
 
Reported: 2011-03-28 09:53 PDT by Mark Finkle (:mfinkle) (use needinfo?)
Modified: 2011-07-14 21:36 PDT (History)
6 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (2.07 KB, patch)
2011-05-25 18:07 PDT, Alex Pakhotin (:alexp)
blassey.bugs: review-
Details | Diff | Splinter Review
Patch v2 (2.76 KB, patch)
2011-05-26 13:02 PDT, Alex Pakhotin (:alexp)
blassey.bugs: review+
Details | Diff | Splinter Review
Patch - ready to be pushed (2.77 KB, patch)
2011-06-16 16:49 PDT, Alex Pakhotin (:alexp)
alex.mozilla: review+
Details | Diff | Splinter Review

Description Mark Finkle (:mfinkle) (use needinfo?) 2011-03-28 09:53:13 PDT
Some special links, mailto and SMS, support a 'body' item that should prepopulate the body of the resulting helper application. This does not seem to be working for SMS on Android. We should make sure we have the code working for any special links that support 'body.

mailto: and sms: are two special links I know about right now. Any others?
Comment 1 Kevin Brosnan [:kbrosnan] 2011-04-04 17:50:04 PDT
Calendar (calendar:)? and instant messaging (xmpp:)?
Comment 2 Alex Pakhotin (:alexp) 2011-05-25 18:05:26 PDT
It looks like that "mailto:" and "sms:" are the only commonly used protocols, which support the "body" item.
"mailto:" already works without any special actions - the URIs with the "body" item are handled properly, so we only need to add support for "sms:".
Comment 3 Alex Pakhotin (:alexp) 2011-05-25 18:07:11 PDT
Created attachment 535248 [details] [diff] [review]
Patch

Handle "body" item in the "sms:" URIs.
Comment 4 Brad Lassey [:blassey] (use needinfo?) 2011-05-25 18:33:27 PDT
Comment on attachment 535248 [details] [diff] [review]
Patch

Review of attachment 535248 [details] [diff] [review]:
-----------------------------------------------------------------

close, just handle the case where there are other uri args

::: embedding/android/GeckoAppShell.java
@@ +770,5 @@
> +                    if (bodyPos >= 0 && bodyPos + 5 < query.length()) {
> +                        String body = query.substring(bodyPos + 5);
> +                        intent.putExtra("sms_body", body);
> +                        // Remove the query part from the URI
> +                        uri = Uri.parse(aUriSpec.substring(0, aUriSpec.indexOf('?')));

what if there are more arguments to the uri than just body? This doesn't seem to handle that
Comment 5 Alex Pakhotin (:alexp) 2011-05-25 18:45:46 PDT
(In reply to comment #4)
> what if there are more arguments to the uri than just body? This doesn't
> seem to handle that

I had this concern too, but if I read the RFC correctly, "body" is the only valid field: http://tools.ietf.org/html/rfc5724#section-2
I also think we wouldn't want to implement a full parser for the query string, unless it's really needed.
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2011-05-25 19:16:29 PDT
its the only reserved argument, but that doesn't mean its the only possible one. You just have to split() the string with "&"
Comment 7 Alex Pakhotin (:alexp) 2011-05-25 19:29:38 PDT
(In reply to comment #6)
> You just have to split() the string with "&"

Hmm... If I just do it with the original encoded URI, not with the query string, that would work, I guess.
Comment 8 Alex Pakhotin (:alexp) 2011-05-26 13:02:47 PDT
Created attachment 535444 [details] [diff] [review]
Patch v2

Handle the cases with additional fields.
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2011-05-26 13:26:29 PDT
Comment on attachment 535444 [details] [diff] [review]
Patch v2

Review of attachment 535444 [details] [diff] [review]:
-----------------------------------------------------------------

If you haven't please test this with an extra arg. sms:5551234567?body=foo&bar=boo and sms:5551234567?bar=boo&body=foo
Comment 10 Alex Pakhotin (:alexp) 2011-05-26 14:29:26 PDT
(In reply to comment #9)
> If you haven't please test this with an extra arg.
> sms:5551234567?body=foo&bar=boo and sms:5551234567?bar=boo&body=foo

Yes, I've tested all possible variants - everything works.

Just want to point out though that those extra arguments actually confuse the standard SMS application, which sometimes cannot find the contact if an extra argument is provided after the phone number.
Comment 11 Alex Pakhotin (:alexp) 2011-06-16 16:49:14 PDT
Created attachment 539938 [details] [diff] [review]
Patch - ready to be pushed

r=blassey
Comment 13 Naoki Hirata :nhirata (please use needinfo instead of cc) 2011-06-22 17:18:27 PDT
Verified:
Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110622 Firefox/7.0a1 Fennec/7.0a1
Device: Thunderbolt
OS: Android 2.2

Note You need to log in before you can comment on or make changes to this bug.