Attachment error for signatures containg callto:skypeid, tel:nbr type hrefs

RESOLVED FIXED in Thunderbird 13.0

Status

Thunderbird
Message Compose Window
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: thomas.vansundert, Assigned: Bienvenu)

Tracking

({regression})

10 Branch
Thunderbird 13.0
x86
Windows 7
regression

Thunderbird Tracking Flags

(thunderbird11+ fixed, thunderbird12+ fixed, thunderbird-esr1011+ fixed)

Details

(Whiteboard: [GS], URL)

Attachments

(4 attachments)

(Reporter)

Description

6 years ago
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.
tracking-thunderbird10: --- → ?
Keywords: qawanted
(Assignee)

Comment 1

6 years ago
Is this a regression from previous versions of Thunderbird?
(Reporter)

Comment 2

6 years ago
It worked perfectly up to v9. The problem started from v10 beta 1.
tracking-thunderbird10: ? → ---
tracking-thunderbird10: --- → ?
(Assignee)

Updated

6 years ago
Assignee: nobody → dbienvenu
(Assignee)

Comment 3

6 years ago
Yes, this is reproducible - I suspect this is a regression from the ResetEmbeddedURI's changes having to do with saving drafts with embedded images.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
tracking-thunderbird10: ? → +
tracking-thunderbird11: --- → +
(Assignee)

Comment 4

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

Comment 5

6 years ago
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.
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
tracking-thunderbird10: + → -
tracking-thunderbird11: + → -
Resolution: --- → INVALID
(Reporter)

Comment 7

6 years ago
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
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(Reporter)

Comment 8

6 years ago
Created attachment 589426 [details]
The reworked attachment

The reworked attachment

Comment 9

6 years ago
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

6 years ago
Sorry number #3 WORKS.
Created attachment 593354 [details]
Problematic Signature

I have the same problem with the attached signature. No tel: but only skype:

Updated

6 years ago
Keywords: qawanted → regression
Summary: Attachment error for signature images when sending a mail → Attachment error for signatures containg callto:skypeid, tel:nbr type hrefs

Updated

6 years ago
Whiteboard: [GS]

Comment 12

6 years ago
The problem is also present with gtalk:

Comment 13

6 years ago
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

6 years ago
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"
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?)
(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
(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

6 years ago
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

6 years ago
"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.

Updated

6 years ago
tracking-thunderbird10: - → ---
tracking-thunderbird11: - → ---
tracking-thunderbird13: --- → ?
(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?
(Assignee)

Comment 21

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

Comment 23

6 years ago
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.
Attachment #599251 - Flags: review?(mbanner)
(Assignee)

Comment 24

6 years ago
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.
tracking-thunderbird11: --- → ?
tracking-thunderbird12: --- → ?
(Assignee)

Comment 25

6 years ago
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!
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!

Updated

6 years ago
Duplicate of this bug: 729442
(Assignee)

Comment 28

6 years ago
Thx Roberto. And I guess your case is different from the bug reporter's.

Comment 29

6 years ago
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!
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

6 years ago
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 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.
Attachment #599251 - Flags: review?(mbanner) → review+
(Assignee)

Comment 33

6 years ago
fixed on trunk - http://hg.mozilla.org/comm-central/rev/d5a16ea57ebb
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
(Assignee)

Comment 34

6 years ago
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.
Attachment #599251 - Flags: approval-comm-release?
Attachment #599251 - Flags: approval-comm-beta?
Attachment #599251 - Flags: approval-comm-release?
Attachment #599251 - Flags: approval-comm-release+
Attachment #599251 - Flags: approval-comm-beta?
Attachment #599251 - Flags: approval-comm-beta+

Comment 35

6 years ago
Is there any chance to see the fix ported to TB10?
Comment on attachment 599251 [details] [diff] [review]
possible fix

[Triage Comment]
Opps, meant to set approval-comm-aurora+ rather than approval-comm-release+
Attachment #599251 - Flags: approval-comm-release+ → approval-comm-aurora+
status-thunderbird-esr10: --- → checkin-pending
tracking-thunderbird11: ? → +
tracking-thunderbird12: ? → +
tracking-thunderbird13: ? → ---
tracking-thunderbird-esr10: --- → 11+
Landed on aurora/beta branches:

http://hg.mozilla.org/releases/comm-aurora/rev/80a80cd502a7
http://hg.mozilla.org/releases/comm-beta/rev/5e996adb1ff5
status-thunderbird11: --- → fixed
status-thunderbird12: --- → fixed
Landed for esr 10.0.3: http://hg.mozilla.org/releases/comm-esr10/rev/348b51b0728c
status-thunderbird-esr10: checkin-pending → fixed
Blocks: 692875
You need to log in before you can comment on or make changes to this bug.