Last Comment Bug 714825 - Attachment error for signatures containg callto:skypeid, tel:nbr type hrefs
: Attachment error for signatures containg callto:skypeid, tel:nbr type hrefs
Status: RESOLVED FIXED
[GS]
: regression
Product: Thunderbird
Classification: Client Software
Component: Message Compose Window (show other bugs)
: 10 Branch
: x86 Windows 7
: -- normal with 8 votes (vote)
: Thunderbird 13.0
Assigned To: David :Bienvenu
:
Mentors:
http://getsatisfaction.com/mozilla_me...
: 729442 (view as bug list)
Depends on:
Blocks: 692875
  Show dependency treegraph
 
Reported: 2012-01-03 09:02 PST by thomas.vansundert
Modified: 2014-03-25 04:47 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
+
fixed
11+
fixed


Attachments
appstrakt_sig.html (4.72 KB, text/plain)
2012-01-03 09:02 PST, thomas.vansundert
no flags Details
The reworked attachment (4.71 KB, text/plain)
2012-01-18 00:42 PST, thomas.vansundert
no flags Details
Problematic Signature (4.94 KB, text/plain)
2012-02-01 02:11 PST, Roberto Ballerini - Automa
no flags Details
possible fix (1.41 KB, patch)
2012-02-21 10:30 PST, David :Bienvenu
standard8: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description thomas.vansundert 2012-01-03 09:02:40 PST
Created attachment 585431 [details]
appstrakt_sig.html

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:10.0) Gecko/20100101 Firefox/10.0
Build ID: 20111228055358

Steps to reproduce:

I have an HTML based signature (see attachment) that also contains remotely hosted images. The signature appear correctly when composing a mail.


Actual results:

When I send the mail or when the mail is automatically saved as a draft, some of the images in my signature disappear and the following message appears:

"Sending of message failed.
Attachment error."

I can not sent the mail unless by removing my signature.


Expected results:

The mail should be sent perfectly and the attachment should look fine.
Comment 1 David :Bienvenu 2012-01-04 08:41:42 PST
Is this a regression from previous versions of Thunderbird?
Comment 2 thomas.vansundert 2012-01-04 23:52:25 PST
It worked perfectly up to v9. The problem started from v10 beta 1.
Comment 3 David :Bienvenu 2012-01-05 08:04:20 PST
Yes, this is reproducible - I suspect this is a regression from the ResetEmbeddedURI's changes having to do with saving drafts with embedded images.
Comment 4 David :Bienvenu 2012-01-17 10:37:06 PST
The issue with the test case seems to be the anchor elements with telephone numbers, e.g.,

										<td><a href="tel:+32  32 498 82 16 84"><img src="http://www.appstrakt.be/mailsignature/crew/thomas/row4_2_cellphone.png" width="134" height="22" style="display:block" border="0" alt="tel:+32  32 498 82 16 84" /></a></td>


That's not a valid href as far as the code is concerned, which causes an error in nsMsgComposeAndSend::GetEmbeddedObjectInfo. I'll try to figure out why the older code doesn't care about the error.
Comment 5 David :Bienvenu 2012-01-17 10:55:10 PST
OK, the change that broke this is in the core necko http code, which is not happy with the url getting passed in. (we stick http:// in front because we recognize that it's not a valid url) I suspect you need to escape the ':' and the spaces to make it happy. My inclination would be to mark this invalid, if Mark agrees.
Comment 6 Mark Banner (:standard8) 2012-01-17 12:25:34 PST
I checked out the RFC for tel: http://tools.ietf.org/html/rfc3966

It lists these characters as visual separators: - . ( )

It doesn't list space. So the code is now being stricter according to the protocol.

Therefore I agree with the invalid resolution.
Comment 7 thomas.vansundert 2012-01-18 00:41:45 PST
I have adapted the signature (I've attached the code) and removed the spaces in the URI. I however still get the same result.

I've noticed that if I double click the image, the "link" field contains "tel:0032-498-82-16-84". I guess this is the problem (it's now indeed http://tel:0032-498-82-16-84). If I remove the link completely (for both phone numbers in the signature) the mail is sent correctly
Comment 8 thomas.vansundert 2012-01-18 00:42:42 PST
Created attachment 589426 [details]
The reworked attachment

The reworked attachment
Comment 9 jun 2012-01-24 07:12:01 PST
I've experienced the same problem with  the error message :
"Sending of message failed.
Attachment error."

I have done the following test with Thunderbird 10 beta 4 :
#1 - Signature with an <img> tag and a <a href="callto:skypeid"> : FAILS
#2 - Signature with just an <img> tag : WORKS
#3 - Signature with just the <a href="callto:skypeid"> : FAILS

Variations on #1 using tel: or skype: also fail. 

Alhtough I understand these are not W3C standard protocols (although tel: seems to be). It is quite common to want to facilitate access to various communication software through this type of notation, particularly in signatures.

For now I will remove the special protocol link but assume this might have to be addressed (or at least precised in a clearer error message). 

Best,

Jun.
Comment 10 jun 2012-01-25 02:51:34 PST
Sorry number #3 WORKS.
Comment 11 Roberto Ballerini - Automa 2012-02-01 02:11:31 PST
Created attachment 593354 [details]
Problematic Signature

I have the same problem with the attached signature. No tel: but only skype:
Comment 12 fglez 2012-02-03 01:04:51 PST
The problem is also present with gtalk:
Comment 13 david woakes 2012-02-03 01:52:04 PST
I can confirm that messages with a callto:skypeid and an image will fail.

Using callto:skypeid or the image in isolation work.

Using moz-do-not-send="true" for the image doesn't help.
Comment 14 Arivald 2012-02-06 05:39:42 PST
I can confirm it too.

If image is before skype link, it works.

One way to make it send mail is to add one "/" to skype URL

bad:
<a href="skype:testUsername?call">TEST</a>

working:
<a href="skype:/testUsername?call">TEST</a>
although Skype will try to call user "/testUsername"
Comment 15 intendentedelleacque 2012-02-07 11:48:16 PST
On Linux I can't see this bug, so I wonder if it depends from some windows-specific change (for example https://bugzilla.mozilla.org/show_bug.cgi?id=22473?)
Comment 16 intendentedelleacque 2012-02-07 14:17:29 PST
(In reply to intendentedelleacque from comment #15)
> On Linux I can't see this bug, so I wonder if it depends from some
> windows-specific change (for example
> https://bugzilla.mozilla.org/show_bug.cgi?id=22473?)

The bug above is wrong, the right link is https://bugzilla.mozilla.org/show_bug.cgi?id=224733
Comment 17 Roberto Ballerini - Automa 2012-02-14 07:55:24 PST
(In reply to david woakes from comment #13)
> 
> Using moz-do-not-send="true" for the image doesn't help.

Using moz-do-not-send="true" in the skype: URI DO help. 
The message is correctly managed by TB, but the skype link doesn't work in Gmail (I suppose because it doesn't understand the attribute).
Comment 18 Andrei Niculae 2012-02-17 06:11:55 PST
You do realize this is a very big bug. Imagine that now all the click to call applications will stop working and everybody will either stick with v9 or use outlook instead. 

A very interesting fact is that I can have a <a href="tel:+123">123</a> but if I also have an <img tag in there, it stops working. 

This looks like the parser is doing something very strange ...

Please solve this.

Original signature (worked in 9):
<html>
<body link="#000000" alink="#ff0000">
-- <br>
<a href="http://www.XXX.com" target="_blank"><img src="file://C:/ProgramData/XXX/BaseImage/obs/XXX.png" alt="XXX" border="0"></a><br>
<font color="#666666" face="Verdana, Arial, Helvetica, sans-serif" size="2">Andrei Niculae | Consultant<br>Phone: <a href="tel:+123">+123</a>| Mobile: <a href="tel:+123">+123</a> 
<br>
<font color="#ff0000">XXX</font> XX<br>
address</font>
<br>
<table>
<tr>
<td><a href="http://www.XXX.com/" target="_blank"><img src="file://C:/ProgramData/XXX/BaseImage/obs/YYY.gif" alt="YYY" width="44" height="28" border="0" align="abscenter"></a></td>
<td><font color="#4B7D42" size="1" face="Verdana, Arial, Helvetica, sans-serif">YYY ZZZ</font></td>
</tr>
</table>
</body>
</html>
Comment 19 alex mateescu 2012-02-17 06:18:10 PST
"Very big bug" is an understatement. This is one of those bugs our clients would fry us for, should we let it slip through testing. Needless to say many of the diehards that stuck with TB despite company's policy, have now switched to Outlook. Because, you know, some signatures are mandatory and can't be edited as a workaround.
Comment 20 WADA 2012-02-18 17:43:23 PST
(In reply to David :Bienvenu from comment #5)
> (we stick http:// in front because we recognize that it's not a valid url)

Why should http:// be sticked in front of not-valid url which starts with unknown-schema-name + ":"?
Why should http:// be sticked in front of callto:skypeid(undefined schema name) or tel:+123(defined schema name)?
Why not-valid url is not ignored?
Who is "we"? Tb? Parser? Necko? Feature like "auto-complement of http:// at URLbar" is culprit?
Comment 21 David :Bienvenu 2012-02-18 19:19:22 PST
(In reply to WADA from comment #20)
> (In reply to David :Bienvenu from comment #5)
> > (we stick http:// in front because we recognize that it's not a valid url)
> 
> Why should http:// be sticked in front of not-valid url which starts with
> unknown-schema-name + ":"?
> Why should http:// be sticked in front of callto:skypeid(undefined schema
> name) or tel:+123(defined schema name)?
> Why not-valid url is not ignored?
> Who is "we"? Tb? Parser? Necko? Feature like "auto-complement of http:// at
> URLbar" is culprit?

See https://bugzilla.mozilla.org/show_bug.cgi?id=714825#c5 - this seems to be a necko change. We can try to ignore the failure, but I'm not sure what will break downstream.
Comment 22 WADA 2012-02-18 19:55:58 PST
Aha!
(i)  Tb adds "http://" if "Invalid URL" is returned from Necko.
(ii) Old Necko returned "Vaild"   to callto:skypeid or tel:+123.
     New Necko returns  "Invaild" to callto:skypeid or tel:+123.

It looks for me that (i) is to avoid further "invalid URL" errors which occurred in the past.
How about next?
> (a) network.protocol-handler.external.Dummy = false.
> (b) Never pass Dummy: to external handler nor Tb's handler(ignore true).
> (c) Add "Dummy://" internally, if "Invalid URL" is returned from Necko.
>     Don't change URL like string in message source.
I think attachment code can ignore this Dummy://.
Comment 23 David :Bienvenu 2012-02-21 10:30:50 PST
Created attachment 599251 [details] [diff] [review]
possible fix

This ignores the error creating the url, and handles have a null attachment url a bit further downstream. With this fix, I can save as draft and send the sig that Thomas attached to this bug. I've requested a try server build as well, so that others can try this and verify it fixes their cases as well. I'll post a link when the try server build starts completing.
Comment 24 David :Bienvenu 2012-02-21 10:32:20 PST
I think we should track this for TB 11 and 12 as well. I think the fix is *relatively* safe and I'd like to know if it fixes the real world cases, which means getting it into alpha and beta builds.
Comment 25 David :Bienvenu 2012-02-22 08:11:32 PST
try server builds here:

http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-52111db5538f/

If this bug is affecting you, and you'd like to see a fix landed sooner rather than later, trying out the build and letting us know if it fixes the issues you're seeing would be very helpful, thx!
Comment 26 Roberto Ballerini - Automa 2012-02-22 09:12:29 PST
Hi David.
I made a test and it sent a message that was blocked on TB10.X.
Thanks

(In reply to David :Bienvenu from comment #25)
> try server builds here:
> 
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> bienvenu@nventure.com-52111db5538f/
> 
> If this bug is affecting you, and you'd like to see a fix landed sooner
> rather than later, trying out the build and letting us know if it fixes the
> issues you're seeing would be very helpful, thx!
Comment 27 rsx11m 2012-02-22 10:55:03 PST
*** Bug 729442 has been marked as a duplicate of this bug. ***
Comment 28 David :Bienvenu 2012-02-22 11:46:17 PST
Thx Roberto. And I guess your case is different from the bug reporter's.
Comment 29 fglez 2012-02-23 01:49:10 PST
I tried this build and I was able to send messages with signature containing a gtalk: URL, which was not possible since TB 10.0.0

Thank you, David.

(In reply to David :Bienvenu from comment #25)
> try server builds here:
> 
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/
> bienvenu@nventure.com-52111db5538f/
> 
> If this bug is affecting you, and you'd like to see a fix landed sooner
> rather than later, trying out the build and letting us know if it fixes the
> issues you're seeing would be very helpful, thx!
Comment 30 Roberto Ballerini - Automa 2012-02-23 02:47:06 PST
Hi David. My signature is the third attachment of this ticket.
Basically it contains a skype: link and various remote images.
(In reply to David :Bienvenu from comment #28)
> Thx Roberto. And I guess your case is different from the bug reporter's.
Comment 31 leonardtheron 2012-02-23 04:49:19 PST
I can confirm I'm pretty sure it's the image embedded in the signature because when I suppress it, the mail can be sent!
Comment 32 Mark Banner (:standard8) 2012-02-24 04:38:47 PST
Comment on attachment 599251 [details] [diff] [review]
possible fix

Ok, this seems to be fine. I looked through the code and it appears we have the right checks in place already for a null attachment->m_url, so I think this is safe.
Comment 33 David :Bienvenu 2012-02-24 12:39:38 PST
fixed on trunk - http://hg.mozilla.org/comm-central/rev/d5a16ea57ebb
Comment 34 David :Bienvenu 2012-02-24 12:40:57 PST
Comment on attachment 599251 [details] [diff] [review]
possible fix

I also had checked other users of attachment->m_url, and fixed the one place we deref it w/o checking.
Comment 35 alex mateescu 2012-02-28 08:27:34 PST
Is there any chance to see the fix ported to TB10?
Comment 36 Mark Banner (:standard8) 2012-02-28 08:28:44 PST
Comment on attachment 599251 [details] [diff] [review]
possible fix

[Triage Comment]
Opps, meant to set approval-comm-aurora+ rather than approval-comm-release+
Comment 38 Mark Banner (:standard8) 2012-03-05 13:52:44 PST
Landed for esr 10.0.3: http://hg.mozilla.org/releases/comm-esr10/rev/348b51b0728c

Note You need to log in before you can comment on or make changes to this bug.