Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat semicolons as a delimiter outside of groups. #5

Merged
merged 1 commit into from
Jan 7, 2015

Conversation

asutherland
Copy link
Contributor

This patch modifies addressparser to treat semicolons like commas when used
outside of a group. I believe this may have been the original intent of
addressparser since there is explicit logic in addressparser.parse that already
treats semicolons identically to commas.

Context: In the Firefox OS email app we allow the user to use the semicolon
character to act as a delimiter in the UI for consistency with other email
client UIs and because there is no valid use for the semicolon apart from a
delimiter in that context. The app currently depends on addressparser to treat
the semicolon as a delimiter identical to a comma when used outside of a group,
but addressparser does not. I think it makes more sense to address this in
addressparser than in our UI in this situation, but we can also do that.

(Note that this was a regression in our UI, but it was probably due to the
change to have our UI use addressparser directly rather than the very naive
approach we were using previously that just looked for commas and spaces and
semicolons.)

This patch modifies addressparser to treat semicolons like commas when used
outside of a group.  I believe this may have been the original intent of
addressparser since there is explicit logic in addressparser.parse that already
treats semicolons identically to commas.

Context: In the Firefox OS email app we allow the user to use the semicolon
character to act as a delimiter in the UI for consistency with other email
client UIs and because there is no valid use for the semicolon apart from a
delimiter in that context.  The app currently depends on addressparser to treat
the semicolon as a delimiter identical to a comma when used outside of a group,
but addressparser does not.  I think it makes more sense to address this in
addressparser than in our UI in this situation, but we can also do that.

(Note that this was a regression in our UI, but it was probably due to the
change to have our UI use addressparser directly rather than the very naive
approach we were using previously that just looked for commas and spaces and
semicolons.)
@andris9
Copy link
Member

andris9 commented Jan 7, 2015

Seems reasonable to me. I have also made the same mistake of using ; as the separator and then wondering why the MUA does not understand the addresses. And about group syntaxes – personally I don't think that I have ever seen any other usage than:

To: undisclosed-recipients:;

And thats about it for my experiences with the group syntax.

andris9 added a commit that referenced this pull request Jan 7, 2015
Treat semicolons as a delimiter outside of groups.
@andris9 andris9 merged commit e5c9c0c into emailjs:master Jan 7, 2015
@asutherland asutherland deleted the semicolons-as-delim branch January 7, 2015 18:52
@asutherland
Copy link
Contributor Author

Yes; except for the bcc scenario there to avoid old sendmail breakage, I think it's an abandoned sort of feature. Having said that, I think Thunderbird's address book may support groups with its support for "lists", but I'm not actually sure about that; that's one of the areas of the code I managed to avoid touching and only hear other people's war stories about!

asutherland added a commit to asutherland/gaia-email-libs-and-more that referenced this pull request Jan 21, 2015
…whose addresses are separated by semicolon, the email can’t be sent successfully. r=andris9

Land emailjs/emailjs-addressparser#5 using the
merge commit.  Tests are in upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants