645729 - Make sure 'body' data is sent along with special links (mailto and sms)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

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

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 [Ex-Mozilla]]

Calendar (calendar:)? and instant messaging (xmpp:)?

Comment 2

[Alex Pakhotin (:alexp)]

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)]

Attachment: Patch

Handle "body" item in the "sms:" URIs.

Comment 4

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

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)]

(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?)]

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)]

(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)]

Attachment: Patch v2

Handle the cases with additional fields.

Comment 9

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

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)]

(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)]

Attachment: Patch - ready to be pushed

r=blassey

Comment 12

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/4b326eca59f8

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified:
Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110622 Firefox/7.0a1 Fennec/7.0a1
Device: Thunderbolt
OS: Android 2.2