Bug 91996

Created attachment 118255 [details] [review]
xdg-email-thunderbird-quoting-2.patch

Apparently the single quotes are needed for multiple recipients. Here is an new version of the patch.

Under what circumstances does this fail?

I think we can assume that callers should properly quote (or escape) arguments passed to xdg-email

I suppose we can adopt this assuming the case I mentioned above still works with this change (does it)?

(In reply to Rex Dieter from comment #2)
> Under what circumstances does this fail?

Not sure exactly what you mean, but without the single quotes this fails:

  xdg-email --to pelle@debian.org --to pelle@dsv.su.se

> I think we can assume that callers should properly quote (or escape)
> arguments passed to xdg-email

Of course. But the single quotes in this case is a Thunderbird peculiarity and not a part of xdg-email's interface.

> I suppose we can adopt this assuming the case I mentioned above still works
> with this change (does it)?

Which case?

Does these cases still work afterward?

xdg-email --to "pelle@debian.org" --to "pelle@dsv.su.se"
or
xdg-email --to 'pelle@debian.org' --to 'pelle@dsv.su.se'

if so, I have no problem taking in this fix.

(In reply to Rex Dieter from comment #4)
> Does these cases still work afterward?
> 
> xdg-email --to "pelle@debian.org" --to "pelle@dsv.su.se"
> or
> xdg-email --to 'pelle@debian.org' --to 'pelle@dsv.su.se'
> 
> if so, I have no problem taking in this fix.

Sure. The quotes are interpreted by the shell in these cases and never passed to xdg-email.

OK, looks like the origins of this goes back to commit,
http://cgit.freedesktop.org/xdg/xdg-utils/commit/?id=31c9f61c608433bd38211bd6c68baac393afa983

to handle cases where there was punctuation (like comma's) in email addresses.  I'll try testing both cases.

This new version of patch doesn't seem to fix things fully, what I see getting sent to thunderbird includes an extra trailing comma,

XDG_UTILS_DEBUG_LEVEL=4 xdg-email --to rdieter@math.unl.edu
...
Running thunderbird -compose "to='rdieter@math.unl.edu,'"

which I think is part of the problem here, digging deeper.

Looks like the comma at the end is presumably to support multiple --to command line args, which doesn't seem to documented as supported.

this would require one to change from:
xdg-email --to pelle@debian.org --to pelle@dsv.su.se
to
xdg-email --to pelle@debian.org,pelle@dsv.su.se

Would you object if I removed it?

(In reply to Rex Dieter from comment #6)
> OK, looks like the origins of this goes back to commit,
> http://cgit.freedesktop.org/xdg/xdg-utils/commit/
> ?id=31c9f61c608433bd38211bd6c68baac393afa983
> 
> to handle cases where there was punctuation (like comma's) in email
> addresses.  I'll try testing both cases.

I think the patch author might be confused. This should be the correct way to pass recipients with commas to xdg-email:

  xdg-email --to "\"Per Olofsson, developer\" <pelle@dsv.su.se>"

You always need to quote recipient names with commas, I'm pretty sure that's in the RFC. The outer quotes in this case are for the shell, only the inner quotes are passed to xdg-email.

(In reply to Rex Dieter from comment #8)
> Looks like the comma at the end is presumably to support multiple --to
> command line args, which doesn't seem to documented as supported.
> 
> this would require one to change from:
> xdg-email --to pelle@debian.org --to pelle@dsv.su.se
> to
> xdg-email --to pelle@debian.org,pelle@dsv.su.se
> 
> Would you object if I removed it?

If it is used by applications, yes I would.

But what about this case:

  xdg-email pelle@debian.org pelle@dsv.su.se

It is clearly documented in the man page. Doesn't it cause the same problem?

Also, I don't get a trailing comma in Thunderbird.

You see it only if you enable debugging, for example,

XDG_UTILS_DEBUG_LEVEL=4 xdg-email --to "pelle@dsv.su.se"
Running thunderbird -compose "to='pelle@dsv.su.se,'"

but I see now that the appended comma is mostly harmless, and thunderbird ignores it in all valid cases I can find.



Applying the patch now (adjusted for recent other commits, mostly the change from 'echo' to explicit '/bin/echo'):

http://cgit.freedesktop.org/xdg/xdg-utils/commit/?id=03577f987730bcbe0a9f25b1f7cb386c8c4ede97

thanks again.