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

VERIFIED FIXED in Firefox 7

Status

Fennec Graveyard
General
P4
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: mfinkle, Assigned: alexp)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 7

Details

Attachments

(1 attachment, 2 obsolete attachments)

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

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Reporter)

Updated

6 years ago
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0next+
Blocks: 647338
Calendar (calendar:)? and instant messaging (xmpp:)?
Priority: -- → P4
Assignee: nobody → alexp
tracking-fennec: 2.0next+ → 6+
(Assignee)

Comment 2

6 years ago
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:".
(Assignee)

Comment 3

6 years ago
Created attachment 535248 [details] [diff] [review]
Patch

Handle "body" item in the "sms:" URIs.
Attachment #535248 - Flags: review?(blassey.bugs)
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
Attachment #535248 - Flags: review?(blassey.bugs) → review-
(Assignee)

Comment 5

6 years ago
(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.
its the only reserved argument, but that doesn't mean its the only possible one. You just have to split() the string with "&"
(Assignee)

Comment 7

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

Comment 8

6 years ago
Created attachment 535444 [details] [diff] [review]
Patch v2

Handle the cases with additional fields.
Attachment #535248 - Attachment is obsolete: true
Attachment #535444 - Flags: review?(blassey.bugs)
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
Attachment #535444 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 10

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

Comment 11

6 years ago
Created attachment 539938 [details] [diff] [review]
Patch - ready to be pushed

r=blassey
Attachment #535444 - Attachment is obsolete: true
Attachment #539938 - Flags: review+
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/4b326eca59f8
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 7
Verified:
Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110622 Firefox/7.0a1 Fennec/7.0a1
Device: Thunderbolt
OS: Android 2.2
Status: RESOLVED → VERIFIED
(Reporter)

Updated

6 years ago
tracking-fennec: 6+ → 7+
status-firefox7: --- → fixed
You need to log in before you can comment on or make changes to this bug.