Last Comment Bug 596797 - moz-do-not-send="true" in HTML signature or pasted HTML gets ignored/removed
: moz-do-not-send="true" in HTML signature or pasted HTML gets ignored/removed
Status: VERIFIED FIXED
[tb31needed]
: regression, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Editor (show other bugs)
: 1.9.2 Branch
: All All
: -- normal (vote)
: mozilla2.0b7
Assigned To: :Ehsan Akhgari (out sick)
:
Mentors:
http://skitch.com/syncopated/ds7g9/th...
: 596936 597973 (view as bug list)
Depends on:
Blocks: CVE-2010-2769
  Show dependency treegraph
 
Reported: 2010-09-15 16:35 PDT by Greg Davidson
Modified: 2010-09-27 02:01 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
needed
.11-fixed
needed
.14-fixed
?


Attachments
Patch (v1) (1.75 KB, patch)
2010-09-20 11:10 PDT, :Ehsan Akhgari (out sick)
bzbarsky: review+
dveditz: approval1.9.2.11+
dveditz: approval1.9.1.14+
Details | Diff | Review
Followup (1.78 KB, patch)
2010-09-21 09:36 PDT, :Ehsan Akhgari (out sick)
bzbarsky: review-
Details | Diff | Review
Followup (1.78 KB, patch)
2010-09-21 11:25 PDT, :Ehsan Akhgari (out sick)
bzbarsky: review+
bzbarsky: approval2.0+
dveditz: approval1.9.2.11+
dveditz: approval1.9.1.14+
Details | Diff | Review

Description Greg Davidson 2010-09-15 16:35:17 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_4; en-us) AppleWebKit/533.18.1 (KHTML, like Gecko) Version/5.0.2 Safari/533.18.5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.9) Gecko/20100825 Thunderbird/3.1.3

I've created an html signature with the following code:
<img src="http://www.yoursole.com/media/images/logo-for-email-signatures.gif" alt="SOLE"  style="display:block; margin: 0 0;" width="140" height="27" border="0" moz-do-not-send="true">

When I compose an email using this signature, the moz-do-not-send attribute is getting removed / ignored. Double-clicking on the image confirms this, since the "Attach this image to the message" checkbox is checked.

When the message is sent, the remote image gets attached automatically (without user intervention), resulting in an unwanted attachment.

The workaround involves, double-clicking the image in the signature and unchecking "Attach this image to the message". Not a huge deal, but less usable than 3.1.2 and earlier.

3.1.2 and before did not behave in this fashion and honoured the moz-do-not-send="true" attribute. 




Reproducible: Always

Steps to Reproduce:
1. Use an html signature with a remote image <img src="http://www.yoursole.com/media/images/logo-for-email-signatures.gif" moz-do-not-send="true" />
2. Compose email
3. Send email
Actual Results:  
The image gets added to the message as an attachment automatically. Recipients of the email see an attachment even when there are no user-added attachments.

Expected Results:  
The moz-do-not-send attribute gets honored (and not ignored). Recipient sees an email with an html signature and working remote image (if their permissions allow that).
Comment 1 Mark Banner (:standard8) 2010-09-16 03:56:09 PDT
*** Bug 596936 has been marked as a duplicate of this bug. ***
Comment 2 :Ehsan Akhgari (out sick) 2010-09-16 12:10:18 PDT
What's the purpose behind this attribute?
Comment 3 Greg Davidson 2010-09-16 22:51:17 PDT
@Ehsan to either download and attach a remote image (moz-do-not-send="false") or to just leave it as a link (moz-do-not-send="true"
Comment 4 Ludovic Hirlimann [:Usul] 2010-09-20 04:02:35 PDT
*** Bug 597973 has been marked as a duplicate of this bug. ***
Comment 5 Gabe Hiemstra 2010-09-20 04:50:26 PDT
the workaround does work, but unfortunately this is out of the question when sending lots of emails every day.

hope this gets fixed soon, because i also feel very attached to the quickfilter bar. so i dont want to switch to previous versions..
Comment 6 rsx11m 2010-09-20 06:28:47 PDT
Confirming Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100915 SeaMonkey/2.1b1pre and moving to Editor. Maybe it's possible to restrict that attribute to MailNews use if it's not wanted elsewhere?On the other hand, it won't do any harm outside that context.This is somewhat different than bug 572637 in that no security violation is reported in the error console.
Comment 7 Gabe Hiemstra 2010-09-20 07:04:02 PDT
(In reply to comment #6)
> Confirming Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100915
> SeaMonkey/2.1b1pre and moving to Editor. Maybe it's possible to restrict that
> attribute to MailNews use if it's not wanted elsewhere?On the other hand, it
> won't do any harm outside that context.This is somewhat different than bug
> 572637 in that no security violation is reported in the error console.

i do not know exactly what you mean, but i do know that moz-do-not-send is an official tag, and because it 'suddenly' doesn't work anymore makes it a bug?

what exactly do you mean with "restrict to mailnews", and "won't do any harm"?
Comment 8 rsx11m 2010-09-20 07:18:41 PDT
That was directed to Ehsan's comment #2, it's a Mozilla-proprietary attribute only having a meaning within the context of mail and news composition, hence my suggestion - if possible - to allow it there but to disallow it in any other context where HTML code is inserted.
Comment 9 Gabe Hiemstra 2010-09-20 07:21:50 PDT
(In reply to comment #8)
> That was directed to Ehsan's comment #2, it's a Mozilla-proprietary attribute
> only having a meaning within the context of mail and news composition, hence my
> suggestion - if possible - to allow it there but to disallow it in any other
> context where HTML code is inserted.

oh i understand now :)

how come the "mailnews.display.html_sanitizer.allowed_tags" cannot override this bug neither?

i tried the following:

img(alt,title,longdesc,src,moz-do-not-send,style)
Comment 10 rsx11m 2010-09-20 07:26:08 PDT
(In reply to comment #9)
> how come the "mailnews.display.html_sanitizer.allowed_tags" cannot override
> this bug neither?

That's the wrong end; this preference only affects the /display/ of HTML messages, the problem occurs during composition already.
Comment 11 Gabe Hiemstra 2010-09-20 07:35:19 PDT
ohh interesting(In reply to comment #10)
> (In reply to comment #9)
> > how come the "mailnews.display.html_sanitizer.allowed_tags" cannot override
> > this bug neither?
> 
> That's the wrong end; this preference only affects the /display/ of HTML
> messages, the problem occurs during composition already.

thanks that explains.

when i reply to my own messages containing a correct external image in signature, and send it to myself again, then it leaves the original good image intact, but the new second image gets messed up.

so basicly by replying on email containing external images, it works perfectly fine, just not when creating a new one.

hope this is of any help.
Comment 12 Daniel Veditz [:dveditz] 2010-09-20 10:44:21 PDT
Presumably Thunderbird 3.0.x also has the same bug since we landed bug 520189 there as well.
Comment 13 Gabe Hiemstra 2010-09-20 10:54:41 PDT
(In reply to comment #12)
> Presumably Thunderbird 3.0.x also has the same bug since we landed bug 520189
> there as well.

sorry i am not authorized to acces that bug... now my eyes are bugged by the red stuff...
Comment 14 Mark Banner (:standard8) 2010-09-20 11:04:44 PDT
Gabe: you don't need to follow everything if you don't understand it. Most of these comments are directed at developers.
Comment 15 :Ehsan Akhgari (out sick) 2010-09-20 11:10:50 PDT
Created attachment 476857 [details] [diff] [review]
Patch (v1)

Please note that this will be NPOTB for Firefox, and it can land on mozilla-central without approval.
Comment 16 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-09-20 11:14:05 PDT
Comment on attachment 476857 [details] [diff] [review]
Patch (v1)

Hmm.  OK, but why not just do that unconditionally?  I'd rather do that than add ifdefs here....  r=me either way, I guess.
Comment 17 :Ehsan Akhgari (out sick) 2010-09-20 14:37:39 PDT
http://hg.mozilla.org/mozilla-central/rev/9fee371c0106
Comment 18 rsx11m 2010-09-21 04:39:36 PDT
This doesn't fix it for me in SeaMonkey tinderbox build #1285032058, Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100920 SeaMonkey/2.1b1pre {BuildID: 20100920182058} which should have the respective fix per http://hg.mozilla.org/mozilla-central/graph/54381

Using the HTML code in the original description as the signature or pasting it directly into Insert > HTML still strips the moz-do-no-send="true" attribute.

Either this needs to be reopened or there is some configuration option missing on TB/SM's side to make the #ifdef effective.
Comment 19 rsx11m 2010-09-21 05:47:46 PDT
The build log shows that -D_HAS_EXCEPTIONS=0 -DMOZILLA_INTERNAL_API
-DMOZ_SUITE=1 -DOSTYPE=\"WINNT5.2\" -DOSARCH=WINNT -D_IMPL_NS_LAYOUT
-DNDEBUG -DTRIMMED -UDEBUG -DNDEBUG -DMOZILLA_CLIENT were provided but not -DMOZ_MAIL_NEWS when compiling mozilla/content/base/src/nsContentSink.cpp
Comment 20 Mark Banner (:standard8) 2010-09-21 06:33:54 PDT
So I've just realised that MOZ_MAIL_NEWS either needs to be defined in Makefile.in for the compilation, or we remove that ifdef as per comment 16. Not having an ifdef may be slightly better for if we end up with pure libxul configuration and haven't migrated yet.
Comment 21 rsx11m 2010-09-21 06:56:54 PDT
As another observation on the patch, just to clarify:

> +GK_ATOM(mozdonotsend, "_moz_do_not_send")

This does match "moz-do-not-send" as well, or just "-moz-do-not-send"?
Note that this attribute doesn't have a leading hyphen.
Comment 22 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-09-21 08:28:06 PDT
Yeah, I think we should just remove the ifdefs (and fix the attr name there, of course!)
Comment 23 :Ehsan Akhgari (out sick) 2010-09-21 09:36:27 PDT
Created attachment 477165 [details] [diff] [review]
Followup

Yes!  This is what happens when you have ADD and do not read comment 0 carefully, and then submit a patch for a product which you do not build locally.  I _think_ I got it right this time, but so did I last time, so it would be great if someone who builds comm-central can test this as well.

That said, with bug 597784, this should no longer be necessary, provided that mailnews is using InsertHTML to inject the signature...
Comment 24 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-09-21 09:58:36 PDT
Comment on attachment 477165 [details] [diff] [review]
Followup

This is still wrong (underscores vs dashes), no?
Comment 25 :Ehsan Akhgari (out sick) 2010-09-21 11:18:44 PDT
Aren't dashes replaces by underscores in atoms?  If not, then yes, this is wrong...
Comment 26 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-09-21 11:23:00 PDT
> Aren't dashes replaces by underscores in atoms?

Not that I know of.  Atoms are just strings.  The atom _name_ would replace dashes with underscores if needed, since you can't have dashes in an identifier.  But the atom value should have dashes.
Comment 27 :Ehsan Akhgari (out sick) 2010-09-21 11:25:46 PDT
Created attachment 477208 [details] [diff] [review]
Followup
Comment 28 :Ehsan Akhgari (out sick) 2010-09-21 12:11:46 PDT
Beltzner, what's the landing strategy for bugs which are branch blockers but not b7 blockers.  According to the new rules, they can't land on m-c, which may cause them to miss the branch code freeze date...
Comment 29 :Ehsan Akhgari (out sick) 2010-09-22 09:15:34 PDT
http://hg.mozilla.org/mozilla-central/rev/f5603541696d
Comment 30 Daniel Veditz [:dveditz] 2010-09-22 11:01:46 PDT
Comment on attachment 477208 [details] [diff] [review]
Followup

Approved for 1.9.2.11 and 1.9.1.14, a=dveditz for release-drivers
Comment 32 rsx11m 2010-09-23 08:46:57 PDT
Verified fixed on trunk for Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100923 SeaMonkey/2.1b1pre, both pasting into Insert > HTML and for signature HTML code with the example provided by the bug opener.
Comment 33 Al Billings [:abillings] 2010-09-23 15:09:06 PDT
Verified fix for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.11pre) Gecko/20100922 Lanikai/3.1.5pre.
Comment 34 Al Billings [:abillings] 2010-09-23 15:14:13 PDT
Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.12) Gecko/20100923 Shredder/3.0.9pre.
Comment 35 OJ Timsen 2010-09-26 00:42:05 PDT
Hi all,
I checked on my Linux Box -> Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.11pre) Gecko/20100922 Shredder/3.1.5pre <- but the argument moz-do-not-send "true" is still ignored after opening a fresh "compose e-mail message" window where the .html signature gets attached automatically.

I also tried with a completely fresh profile generated by this release of TB - but still the argument gets ignored when set to true.
Any known workaround besides of editing the image manually in every e-mail?

Thank you and good day to all!

TJ
Comment 36 rsx11m 2010-09-26 08:31:52 PDT
> (comment #35) Linux Box -> Mozilla/5.0 (X11; U; Linux i686; en-US;
> rv:1.9.2.11pre) Gecko/20100922 Shredder/3.1.5pre <- but the argument

Since the patch was pushed Wed Sep 22 13:27:12 2010 -0700, it wouldn't be available in the September 22 nightly builds yet. Did you try a later build?
Comment 37 OJ Timsen 2010-09-26 11:27:48 PDT
Hi rsx,
thank you for your reply.

I tried with the TB release from http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-1.9.2/ which was build last night. I thought this is the newest version.
I also recognized that the argument for the "Alternate Text" field of an image is not imported correctly from a html signature, or better said TB lost the value set and saved in the html signature.
Only workaround yet is to compose a new message, double click the image (in the signature) and edit the "Alt Text" and to unmark the checkbox for "Attach this image to the message" (moz-do-not-send).

Can somebody else reproduce this at the moment? 

Regards, Tim
Comment 38 rsx11m 2010-09-26 14:59:27 PDT
Since this bug is closed, can you open a new report on any other necessary attribute which can no longer be included due to bug 520189?
Comment 39 OJ Timsen 2010-09-26 23:32:13 PDT
working with following version -> Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.11pre) Gecko/20100926 Lanikai/3.1.5pre

Tested a few minutes ago. Now TB gets all attributes correctly and the signature gets delivered as defined within template.

Thank you rsx. Tim

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