B2G MMS: support email addressing model

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Device Interfaces
--
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vicamo, Assigned: vicamo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla17
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
The MMS Addressing Model[1] lists four possible categories: email, device-address, alphanum-shortcode, and num-shortcode. Current implementation in bug 749856 uses "unknown" for email addresses.

Updated

5 years ago
blocking-basecamp: --- → ?

Comment 1

5 years ago
MMS is desired to be ready for basecamp, but does not actually block.
blocking-basecamp: ? → -

Comment 2

5 years ago
MMS is requred for v1, but not for Brazil
blocking-basecamp: - → ?
(Assignee)

Updated

5 years ago
Assignee: nobody → vyang
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Created attachment 645199 [details] [diff] [review]
v0
Attachment #645199 - Flags: review?(philipp)
(Assignee)

Comment 4

5 years ago
Created attachment 645220 [details] [diff] [review]
v1

break the long long regex down.
Attachment #645199 - Attachment is obsolete: true
Attachment #645199 - Flags: review?(philipp)
Attachment #645220 - Flags: review?(philipp)
Comment on attachment 645220 [details] [diff] [review]
v1

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

::: dom/mms/src/ril/MmsPduHelper.jsm
@@ +97,5 @@
> +      // 3.4 of RFC2822, but excluding the obsolete definitions as indicated by
> +      // the "obs-" prefix. Here we match only `addr-spec`.
> +      let dotatom = "[a-zA-Z\d!#$%&'*+\-/=?^_`{|}~]+";
> +      let addrspec = new RegExp(dotatom + "(?:" + dotatom + ")*@" +
> +                                dotatom + "(?:" + dotatom + ")*");

I'm not sure I'm comfortable with validating the email address with such a specific RegExp. I can think of several invalid email addresses that would pass this test. Conversely I bet there are valid addresses that wouldn't. Maybe we can just check for one and only one "@" character in the string, and leave the validation up to the email protocol layer?

(Also, it's unnecessary to compile the RegExp every time we pass through this code. I would suggest a lazy global, if we were to keep it.)
Attachment #645220 - Flags: review?(philipp)
(Assignee)

Comment 6

5 years ago
Created attachment 645704 [details] [diff] [review]
v2

Match a '@' char instead and maybe come back for more strict/completed checks if necessary. Thanks Philipp.
Attachment #645220 - Attachment is obsolete: true
Attachment #645704 - Flags: review?(philipp)
Attachment #645704 - Flags: review?(philipp) → review+
(Assignee)

Comment 7

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/fec33ba83a18
Target Milestone: --- → mozilla17

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/fec33ba83a18
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
blocking-basecamp: ? → +
You need to log in before you can comment on or make changes to this bug.