Closed
Bug 192557
Opened 21 years ago
Closed 20 years ago
Each instance of editing an HTML draft/template copies the <head> to the <body>
Categories
(MailNews Core :: Composition, defect)
MailNews Core
Composition
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: colin, Assigned: neil)
References
Details
(Whiteboard: fixed-aviary1.0)
Attachments
(2 files, 1 obsolete file)
6.44 KB,
patch
|
glazou
:
review+
mscott
:
superreview+
|
Details | Diff | Splinter Review |
4.03 KB,
patch
|
Brade
:
review+
mscott
:
superreview+
sspitzer
:
approval1.8a2+
|
Details | Diff | Splinter Review |
I have a mail message in my Drafts folder which I keep editing. I add stuff and I delete stuff, and overall the number of characters in the mail doesn't change much. But over time the size of the mail message continues to grow. Originally it was 9K, and now it is 74K, even thought the actual text is no bigger. It would appear that each time I edit the message it gets another: <meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"> <title></title> because there must be 200 or so instances of these two lines. After all these meta and titles I have about 500 "blank lines", only each "blank" line consists of about 72 spaces. Finally I get to the HTML containing the text of my mail. And interleaved in with the text are more 72-char "blank" lines. Where is all this crud coming from, and is there a way to force the HTML to get rewritten?
Comment 1•21 years ago
|
||
see also bug #70728 this sounds like an editor issue, so cc jfrancis.
Component: Mail Window Front End → Composition
Depends on: 70728
Comment 2•21 years ago
|
||
Note that the superfluous <meta> and <title> tags are appearing in the <body> of the message, not the <head>. (Problem still occurring in 1.6b.)
Comment 3•20 years ago
|
||
As a result of investigating bug 234210, I've done a little more research into this bug. What is happening is that every time you open an HTML template or draft for editing, all of the contents of the <head> are moved into the <body> and the <head> is reinitialized to contain the <meta> and the empty <title>. If you open a draft repeatedly, then there are multiple copies of the <meta> and empty <title> tag in the <body>, as well as the (most recent) one in the <head>. For those intrepid souls who manage to create a template with a <link> or <style> in the <head>, this problem short-circuits their efforts to make a valid HTML document in their mail. In addition, the symptom in bug 234210 manifests: any http: URLs contained within the <style> are subject to "url recognition" and morphed into a cid: URL. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040402 Updating summary.
Blocks: HTML-compose-tracker
OS: Windows 98 → All
Hardware: PC → All
Summary: Editing a draft causes it to continually grow in size → Each instance of editing an HTML draft/template copies the <head> to the <body>
Assignee | ||
Comment 4•20 years ago
|
||
Assignee: sspitzer → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•20 years ago
|
||
Attachment #145570 -
Attachment is obsolete: true
Assignee | ||
Comment 6•20 years ago
|
||
Comment on attachment 145572 [details] [diff] [review] Also remove unused methods aBuf appears to be the HTML document template so instead of appending to the empty editor and fixing up the body tag I just replace everything.
Attachment #145572 -
Flags: superreview?(mscott)
Attachment #145572 -
Flags: review?(daniel)
Comment 7•20 years ago
|
||
Neil do simple tests like applying an HTML background to message then saving as a template or draft still work correctly after this patch (those styles might be on a body element so we may be ok)? Just looking for possible gotchyas...
Comment 8•20 years ago
|
||
This also occurs if you 'edit as new' a saved message that was not saved as a template.
Assignee | ||
Comment 9•20 years ago
|
||
Actually for the test I used this as my template: <html> <head> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <style>th { background: url(about:logo); }</style> </head> <body bgcolor="#000000" text="#ffffff"> <br> </body> </html> because I wanted to see what the URL rewriter would do to my inline url, but that's another bug ;-)
I asked Neil to do a few torture tests with his patch before giving my r=. 1. apply a red bg to the doc before saving it as a template; reopen the template 2. apply a style attribute to the body before saving; is it preserved ? (important for the future of templates) ...
Assignee | ||
Comment 11•20 years ago
|
||
Hmm... that's a bad test, MsgComposeCommands.js stomps on the text and bgcolor attributes in loadHTMLMsgPrefs - in fact if you have the select quote preference combined with font preferences then the entire quote gets reformatted. And that's not the only existing bug. A manual style set on the body persists after the compose window is recycled, the cure is to open a second window.
Comment 12•20 years ago
|
||
(In reply to comment #9) > Actually for the test I used this as my template: > <html> > <head> > <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> > <title></title> > <style>th { background: url(about:logo); }</style> > </head> > <body bgcolor="#000000" text="#ffffff"> > <br> > </body> > </html> > because I wanted to see what the URL rewriter would do to my inline url, but > that's another bug ;-) May I ask how you got the style into the head section. If you did this within message compose, you did something I could not do in 16 or so hours of trying.
Assignee | ||
Comment 13•20 years ago
|
||
Ahem... I edited the Templates file in a text editor :-) But without this (or a similar) patch whenever you edit the template in message compose the style will end up back in the body again...
Comment 14•20 years ago
|
||
(In reply to comment #11) > Hmm... that's a bad test, MsgComposeCommands.js stomps on the text and bgcolor > attributes in loadHTMLMsgPrefs Bug 234354? > And that's not the only existing bug. A manual style set on the body persists > after the compose window is recycled, the cure is to open a second window. Bug 234436. See also bug 147645. Does the patch address any of these addtional bugs? Where did that function RebuildDocumentFromSource() come from, anyway? It only shows up as defined, not used, in LXR.
Assignee | ||
Comment 15•20 years ago
|
||
(In reply to comment #14) >Bug 234354? >Bug 234436. See also bug 147645. >Does the patch address any of these addtional bugs? No, it does not, although I might be able to address those bugs now that I am aware of them. >Where did that function RebuildDocumentFromSource() come from, anyway? It's used by the web page editor JS to switch from source to normal view.
Comment 16•20 years ago
|
||
Comment on attachment 145572 [details] [diff] [review] Also remove unused methods I ran with this for a while and it looks good to me.
Attachment #145572 -
Flags: superreview?(mscott) → superreview+
Updated•20 years ago
|
Comment 17•20 years ago
|
||
Daniel Glazman, would you please make time to review Neil's patch on this bug? The "torture tests" you suggested in comment 10 aren't really related to this bug; each of those items is a known problem with its own bug already: - HTML color/bgcolor are overwritten by prefs (bug 234354) - Inline style is lost (bug 147645 comment 3) Neil, do you see any reason not to remove the dependency on bug 70728?
Assignee | ||
Comment 18•20 years ago
|
||
Hmm... I overlooked the blank lines issue...
Comment on attachment 145572 [details] [diff] [review] Also remove unused methods Seems ok to me if you accept that for instance the bgcolor is not updated if you changed the default bgcolor between your "Save as Draft" and the next "Edit as new"... r=daniel@glazman.org
Attachment #145572 -
Flags: review?(daniel) → review+
Assignee | ||
Comment 20•20 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Whiteboard: fixed-aviary1.0
Comment 21•20 years ago
|
||
Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040610 Both the reporter's original problem, and its root cause cited in comment 3, have been fixed. Thanks, Neil! (And thanks, Daniel, for responding.)
Status: RESOLVED → VERIFIED
Comment 22•20 years ago
|
||
This patch causes a bad regression preventing mail from handling mailto urls anymore. If you pass the compose window a mailto url with a body attribute, we no longer insert the body into the compose window. This is becasue the new method this code uses: htmlEditor->RebuildDocumentFromSource(aBuf); bails if the HTML to be inserted doesn't contain a proper body AND head tag. See: http://lxr.mozilla.org/mozilla/source/editor/libeditor/html/nsHTMLEditor.cpp#1834 re-opening for re-evaluation to address the regression.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Whiteboard: fixed-aviary1.0
Comment 23•20 years ago
|
||
(In reply to comment #22) > If you pass the compose window a mailto url with a body attribute, we no > longer insert the body into the compose window. Hmm... sounds similar to bug 246579.
Assignee | ||
Comment 24•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #150948 -
Flags: superreview?(kinmoz)
Attachment #150948 -
Flags: review?(daniel)
Comment 25•20 years ago
|
||
Neil, check out bug 247313, and consider how the rebuilding of the document affects an HTML reply to its attachment 151017 [details].
Comment 26•20 years ago
|
||
Bug 248219 (mail/news) both seem to be the result of the regression induced by the patch for this bug. Maybe also bug 247817 (TB).
Comment 27•20 years ago
|
||
Also, in comment 22, Scott is referring to bug 246595. (For those keeping track...)
Comment 28•20 years ago
|
||
Requesting blocking because of the bugs cascading off of this.
Flags: blocking1.8a2?
Comment on attachment 150948 [details] [diff] [review] Make RebuildDocumentFromSource smarter >+ if (foundhead) { >+ if (foundclosehead) >+ res = ReplaceHeadContentsWithHTML(Substring(beginhead, beginclosehead)); >+ else if (foundbody) >+ res = ReplaceHeadContentsWithHTML(Substring(beginhead, beginbody)); >+ else >+ res = ReplaceHeadContentsWithHTML(Substring(beginhead, endtotal)); This last line and.... >+ } else { >+ nsReadingIterator<PRUnichar> begintotal; >+ aSourceString.BeginReading(begintotal); >+ NS_NAMED_LITERAL_STRING(head, "<head>"); >+ if (foundclosehead) >+ res = ReplaceHeadContentsWithHTML(head + Substring(begintotal, beginclosehead)); >+ else if (foundbody) >+ res = ReplaceHeadContentsWithHTML(head + Substring(begintotal, beginbody)); >+ else >+ res = ReplaceHeadContentsWithHTML(head); that one scare me a bit. I remind you that both HEAD and BODY start and end tags are optional in HTML. So since your code is not detecting the first non-sub-head element, you could end up erroneously placing all document's contents inside head.
Comment 30•20 years ago
|
||
Inline Forwarding of TEXT/PLAIN messages is broken also. (tested in build 2004070208)
Assignee | ||
Comment 31•20 years ago
|
||
Comment on attachment 150948 [details] [diff] [review] Make RebuildDocumentFromSource smarter Now that I've fixed bug 249665 I've been able to throughly test this in 4 cases: * Forward Inline of either text or HTML * mailto: with a body parameter * send link/page * using additional patch not provided here, switch web page editor from source view to normal view without requiring head or body substrings. Since glazou is now on holiday I'm asking cmanske for review, but he might want to take glazou's comments into account.
Attachment #150948 -
Flags: review?(daniel) → review?(cmanske)
Updated•20 years ago
|
Flags: blocking1.8a2? → blocking1.8a2-
Comment 32•20 years ago
|
||
Comment on attachment 150948 [details] [diff] [review] Make RebuildDocumentFromSource smarter r=brade (with additional comments added per irc conversation)
Attachment #150948 -
Flags: review?(cmanske) → review+
Assignee | ||
Updated•20 years ago
|
Attachment #150948 -
Flags: superreview?(kinmoz) → superreview?(mscott)
Updated•20 years ago
|
Attachment #150948 -
Flags: superreview?(mscott) → superreview+
Assignee | ||
Comment 33•20 years ago
|
||
Comment on attachment 150948 [details] [diff] [review] Make RebuildDocumentFromSource smarter Fixes visible regressions (send page/link, forward inline, some mailto: links)
Attachment #150948 -
Flags: approval1.8a2?
Comment 34•20 years ago
|
||
Comment on attachment 150948 [details] [diff] [review] Make RebuildDocumentFromSource smarter a=sspitzer
Attachment #150948 -
Flags: approval1.8a2? → approval1.8a2+
Assignee | ||
Comment 35•20 years ago
|
||
Fix to fix checked in [crosses fingers]
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Comment 36•20 years ago
|
||
Confirming that the mailto-with-subject, Send Page/Send Link and inline-forward of text/plain is working as expected, with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040710 That addresses bug 248219 and most of bug 246579. Additionally, the gross rewriting of the HTML source on an HTML reply (as was seen on the attachment for bug 247313) is no longer a problem (altho that bug's issue still exists). In Mozilla Mail/News, I am not seeing the symptom of bug 247817 (loss of <img> on reload of draft), but that was reported under TB, which I haven't tested. Inline forwarding of HTML messages is still missing the separator and headers table: bug 250291. The original problem of the <head> data being copied into the <body> is *still* fixed! Verifying.
Status: RESOLVED → VERIFIED
Comment 37•20 years ago
|
||
Still seeing head data repeated in body with TB version 0.7+ (20040711) 20040613 branch build fixed this issue. It would seem that this fix is no longer in the branch builds. Will try a trunk build.
Comment 38•20 years ago
|
||
Test current trunk build TB version 0.6+ (20040706) Extra meta data in body (this bug) is fixed in trunk Escape problems after html edit as per http://bugzilla.mozilla.org/show_bug.cgi?id=224733 was not fixed with this patch.(nor was it expected) Thanks for hanging in there on this fix Neil, this alone will save me many keystrokes.
Comment 39•20 years ago
|
||
Just noticed that trunk version was too old, will re-check for new trunk with this fix
Comment 40•20 years ago
|
||
FYI: I just checked both the original patch and the regression fix back into the aviary 1.0 branch now that the regression has been fixed. Joe, can you test tomorrow's bits?
Whiteboard: fixed-aviary1.0
Comment 41•20 years ago
|
||
Confirming Mike's observations in comment 36 are identical in Tbird version 0.7+ (20040722) http://bugzilla.mozilla.org/show_bug.cgi?id=247817 has been marked a dup of http://bugzilla.mozilla.org/show_bug.cgi?id=224733 and was not affected by this fix. This is the next biggest deterrent to the html user (in my opinion) Additionally while this fix allows styles and scripts to be placed in the head section where some say they should be, 'ampersand' and 'greater than' are still being escaped and not being preserved within the script tags, even if the script is placed in the head. http://bugzilla.mozilla.org/show_bug.cgi?id=229122
Comment 42•20 years ago
|
||
I was mistaken in thinking that bug 247817 was related to this bug, sorry for the misdirection.
Comment 43•20 years ago
|
||
Bug 147645 appears to have been resolved -- could it have been from the fix to this bug?
Comment 44•20 years ago
|
||
any reason to take this for 1.7?
Comment 45•20 years ago
|
||
Here's one good reason: User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.2) Gecko/20040804 Netscape/7.2 (ax) Escalade X-Accept-Language: en-us, en This is a multi-part message in MIME format. --------------070801080104010704090209 Content-Type: text/html; charset=us-ascii Content-Transfer-Encoding: 7bit <!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> </head> <body alink="#0000ff" background="cid:part8.05010401.00090508@xxx.com" bgcolor="#ffffff" link="#ff0000" text="#ff0000" vlink="#800080"> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> <title></title> The above would not be in my inbox. I have been using a version of mail/news with this patch in since it was checked into trunk with no ill-effects whatsoever.
Comment 46•20 years ago
|
||
This is a good fix. Anyone want to put a full patch for the 1.7 branch together?
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•