<title> value of HTML signature file shows up in signature

RESOLVED FIXED in mozilla2.0b4

Status

()

Core
Editor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: u331436, Assigned: Ehsan)

Tracking

(4 keywords)

Trunk
mozilla2.0b4
x86
Windows XP
regression, testcase, verified1.9.1, verified1.9.2
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+, blocking1.9.2 .9+, status1.9.2 .9-fixed, blocking1.9.1 .12+, status1.9.1 .12-fixed)

Details

(Whiteboard: [tb31needed][tb3needed])

Attachments

(3 attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100615 Lightning/1.1a1pre SeaMonkey/2.0pre - not Firefox
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100615 Lightning/1.1a1pre SeaMonkey/2.0pre - not Firefox

when composing a new message (IMAP account) in HTML mode, the word "signature" is inserted before the signature itself.
My signature is a file (signature.html) configured to be picked as a default sig for this account.
This problem appear as a regression between Friday June 11th and Sunday 13th for the SM2.1 nightlies.

Reproducible: Always

Steps to Reproduce:
1.click on compose
2.new composition window opens, in HTML mode (as configured) with the word signature inserted
3.The word "signature" appear with the variable width default font, prior to the signature itself.


Expected Results:  
only the content of the signature file should appear

workaround: edit all outgoing message to remove the word.... annoying !
(Reporter)

Updated

7 years ago
Version: unspecified → Trunk
(Reporter)

Comment 1

7 years ago
will check on test profile
(Reporter)

Comment 2

7 years ago
Same issue with test profile
I created a test HTML signature file in Composer (SM's HTML editor component). After some testing I found that the value inside <title> is displayed in the MailNews Compose window where the HTML file is included as the signature (in HTML mode, of course). Could you please check whether the same applies to you (i.e. does "signature" appear somewhere in your HTML signature file, e.g. the <title>?).
(Reporter)

Comment 4

7 years ago
Yessss ! spot on !
Without anything between the <title></title> (or without any <title> line in the code) the composition window is fine !
That is an easy workaround.
Should this bug be closed here ?
BTW, I am not sure that the bug should not be corrected anyway as Kompozer for instance ask for a title when saving an HTML file.
Confirming and adjusting summary. I haven't checked whether this is a regression yet but it sure is a bug; <title> information shouldn't appear in the signature when composing a message. If it is a regression, then maybe from the recent changes in the editor component, or maybe fallout from the HTML5 parser (which is on by default for some time already).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: HTML composition of message insert the word "signature" before sig → <title> value of HTML signature file shows up in signature
Last-known-good: 2010-06-14 -> regression.

Candidates:
http://hg.mozilla.org/comm-central/rev/db911c5871d2
http://hg.mozilla.org/mozilla-central/rev/208d7f999037
(maybe more)

I'd put my bet on the second one since the first one was only BR handling changes (apart from test changes).

I wonder whether TB is affected, too (would be a surprise if not).
Keywords: regression
Same with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100619 Shredder/3.2a1pre -> over to MailNews Core, although this might actually be a Core bug.
Component: MailNews: Composition → Composition
Product: SeaMonkey → MailNews Core
QA Contact: mailnews-composition → composition
How do you actually insert the signature in the email body editable field?
(In reply to comment #8)
> How do you actually insert the signature in the email body editable field?

SM/TB does that automatically for you if you select an HTML file in Account Settings/<account>/"Attach the signature from a file instead". To reproduce this bug, select a file with these contents there (I hope Bugzilla won't transform this, otherwise: mainly a title tag with "title" and a body tag with "body" as contents):

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
    <title>title</title>
  </head>
  <body>
    body
  </body>
</html>

While this bug is present, you'll see "\n\n-- \ntitle body" when you compose a new message for the modified account.
Hrm, no, I mean, how, as in, what API does the program use to do this!  :-)
htmlEditor->InsertHTML, since we're talking about HTML mode here.
http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp
nsMsgCompose::ConvertAndLoadComposeWindow
nsMsgCompose::SetIdentity
Does the title element itself get added to the mail document as well, or is it just the text node?

By the way, I think all 1.9.1 and 1.9.2 branch nightlies should also be affected by this.  Can someone please confirm?  If that is the case, this bug needs to block the next release of both branches.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Blocks: 520189
Mail bodies (source):

Latest Shredder 3.0 nightly:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>

<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
</head>
<body bgcolor="#ffffff" text="#000000">
<br>
<div class="moz-signature">-- <br>
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
<title>title</title>
body </div>
</body>

</html>

Latest Shredder trunk nightly, html5.enable = true (default):

<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <div class="moz-signature">-- <br>
      title body </div>
  </body>
</html>

Latest Shredder trunk nightly, html5.enable = false:

<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <div class="moz-signature">-- <br>
      title body </div>
  </body>
</html>

Haven't checked TB 3.1.
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Whiteboard: [tb31needs][tb3needs]

Updated

7 years ago
blocking1.9.1: ? → .12+
blocking1.9.2: ? → .9+
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Keywords: testcase-wanted

Updated

7 years ago
Assignee: nobody → ehsan
Created attachment 464140 [details]
Test case
Assignee: ehsan → nobody
Component: Composition → Editor
Keywords: testcase-wanted → testcase
Product: MailNews Core → Core
QA Contact: composition → editor
Created attachment 464254 [details] [diff] [review]
Patch (v1)

We were failing to honor mIgnoreNextCloseHead in the paranoid fragment sink.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #464254 - Flags: review?(roc)
Created attachment 464256 [details] [diff] [review]
Fix a mistake

I also discovered some mistakes in the existing test, which is fixed by this patch.
Attachment #464256 - Flags: review?(roc)
I'm not a content peer and shouldn't be reviewing these patches, sorry
Comment on attachment 464254 [details] [diff] [review]
Patch (v1)

(bz: a quick review here is appreciated as this is a branch blocker)
Attachment #464254 - Flags: review?(roc) → review?(bzbarsky)
Attachment #464256 - Flags: review?(roc) → review?(bzbarsky)
Why is the AddLeaf change needed?
Comment on attachment 464256 [details] [diff] [review]
Fix a mistake

r=me
Attachment #464256 - Flags: review?(bzbarsky) → review+
(In reply to comment #19)
> Why is the AddLeaf change needed?

AddLeaf is what adds the text node under title to the document fragment.
So why does the normal fragment parser not need that?
blocking2.0: ? → final+
(In reply to comment #22)
> So why does the normal fragment parser not need that?

Because in case of the normal fragment parser, the title element would still be added, and will just be popped off in CloseContainer when </head> is encountered.  But <title> is not in the whitelist for the paranoid fragment sink, so the title text ends up getting added as a direct child of the fragment root.
Comment on attachment 464254 [details] [diff] [review]
Patch (v1)

OK, great.  Now can we get all that into a comment in the code? ;)

I hope there are no other whitelisted leaves that we care about, btw...
Attachment #464254 - Flags: review?(bzbarsky) → review+
(In reply to comment #24)
> Comment on attachment 464254 [details] [diff] [review]
> Patch (v1)
> 
> OK, great.  Now can we get all that into a comment in the code? ;)

Of course!  I'll do that before landing.

> I hope there are no other whitelisted leaves that we care about, btw...

I tried to think of others, but couldn't come up with any.  I really hate this part of the code base, BTW.  It has been extremely hard for me to analyze all the cases before actually seeing bugs coming from users, so I hope this is the last regression that I'm fixing here.  :-)
The bug fix itself:

http://hg.mozilla.org/mozilla-central/rev/0aa4ad290221

The test fix:

http://hg.mozilla.org/mozilla-central/rev/40ad71a60486

The comment addition (forgot to add it to the original changeset):

http://hg.mozilla.org/mozilla-central/rev/1b12ed9d1c48
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Attachment #464254 - Flags: approval1.9.2.9?
Attachment #464254 - Flags: approval1.9.1.12?
the test fix light up Md3:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1281492510.1281494162.8440.gz
All three changesets were backed out for the leaks.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 29

7 years ago
Comment on attachment 464254 [details] [diff] [review]
Patch (v1)

a=LegNeato for 1.9.2.9 and 1.9.1.12.

Please note that for these releases code-freeze is this Thursday, 2010-08-12 @ 11:59 pm PST. This needs to be landed as soon as possible.
Attachment #464254 - Flags: approval1.9.2.9?
Attachment #464254 - Flags: approval1.9.2.9+
Attachment #464254 - Flags: approval1.9.1.12?
Attachment #464254 - Flags: approval1.9.1.12+
Pushed the code fixes again...

http://hg.mozilla.org/mozilla-central/rev/962cb94339bf
http://hg.mozilla.org/mozilla-central/rev/962cb94339bf
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8e87b7d03a1f
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8959f16ad2a7
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
status1.9.1: wanted → .12-fixed
status1.9.2: wanted → .9-fixed
Resolution: --- → FIXED
Filed bug 586436 to get the test fixes in as well.
Verified for 1.9.1 and 1.9.2 with passing mochitest.
Keywords: verified1.9.1, verified1.9.2
Whiteboard: [tb31needs][tb3needs] → [tb31needed][tb3needed]
You need to log in before you can comment on or make changes to this bug.