Closed Bug 23931 Opened 25 years ago Closed 24 years ago

HTML InsertAsQuotation() seems to be cutting of HTML input

Categories

(Core :: DOM: Editor, defect, P3)

defect

Tracking

()

VERIFIED WORKSFORME

People

(Reporter: rhp, Assigned: harishd)

References

()

Details

(Whiteboard: [PDT-] plus for beta2)

Attachments

(3 files)

Hi Akkana,
Can you look at the third message in this test folder:

    http://rocknroll/users/momoi/publish/seamonkey/smoketest.zip

If you reply to the third message into a HTML or plain text editor, it seems to
cutoff after the horizontal bar. Just wondering if you could help see where
this is getting cutoff.

- rhp
Target Milestone: M15
I'm setting this to m15 for now, Akkana feel free to move this as you find
appropriate.
I'm willing to help look at this, but Thursday's build didn't work at all, and
when I try to reply to the third message using Wednesday's build, I see
Assertion failure: obj->closed_p, at mimetext.cpp:166
and then the app crashes.  When I have a build from today ready, I'll try again.

Have you tried just printing out what's being passed to
nsHTMLEditor::InsertAsQuotation?
Assignee: akkana → rhp
nsHTMLEditor::InsertAsCitedQuotation is called once, and here is (in ascii) what
is in aQuotedText (only 347 chars):

<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
...SJIS.......
<br>&nbsp;
<p><A
HREF="http://home.netscape.com/ja/">http://home.netscape.com/ja/</A></html>
<BASE HREF="http://home.netscape.com/ja/"><HR WIDTH="90%" SIZE=4><HTML>
<HEAD>
<TITLE>Netcenter <META HTTP-EQUIV="Content-Type" CONTENT="text/html;
charset=x-sjis</html>

Nothing else.  The editor is inserting what it's given; the problem must be in
the code which calls InsertAsQuotation.  Back to you, Rich.
Status: NEW → ASSIGNED
Summary: InsertAsQuotation() seems to be cutting of HTML input → [BETA1] InsertAsQuotation() seems to be cutting of HTML input
I don't see how this could wait till M15. This problem extends to both plain/HTML mail reply. 
If there is an inline attachment, none is quoted of it.  This is definitely a beta 1 material and
needs to get resolved ASAP. This was working not so long ago and is also a regression,
Setting it to M14.
Target Milestone: M15 → M14
Assigning myself as QA as this also involves intl data and we need it
to be working for Beta 1.
QA Contact: sujay → momoi
Hi Akkana,
Update your tree and then go into the file:

  \mozilla\mailnews\compose\src\nsMsgCompose.cpp

and search for the string "Akkana". You'll see an #ifdef 0. Just change that to 
1 and rebuild. This will call a routine that just dumps the content being 
quoted to stdout. From what I can tell, the third message which is the Japanese 
Netcenter home page, is quoting a bunch of HTML, but it is getting truncated 
after insertion.

Thanks for the help!

- rhp
Assignee: rhp → akkana
Status: ASSIGNED → NEW
If I turn on your debug print, and I also insert a debug print into
InsertAsQuotation, this is what I see:

RECEIVE CALLBACK: OnHeadersReady
...SJIS.......
 

http://home.netscape.com/ja/

------------------------------------------------------------------------

InsertAsQuotation(...SJIS.......
 

http://home.netscape.com/ja/

------------------------------------------------------------------------

)

I don't see any characters after the horizontal line.
If I break in nsHTMLEditor::InsertAsQuotation, I see a aQuotedText.Length() is
122, which exactly corresponds to what was printed out, above.

Back to you, Rich.  You still haven't said what you see on stdout, or what the
string length is on your system ...
Assignee: akkana → rhp
Are you trying with an HTML editor?

- rhp
Assignee: rhp → akkana
Summary: [BETA1] InsertAsQuotation() seems to be cutting of HTML input → [BETA1] HTML InsertAsQuotation() seems to be cutting of HTML input
No, because your description explicitly said "a HTML or plain text editor", and
momoi's comment said the same thing.  Sounds like html reply has been fixed to
pass the right info since the last time I tried this, but plaintext reply has
not been fixed yet.

In html compose, I do see the problem -- we are not inserting everything that's
passed in to InsertAsQuotation -- and will look at it.  Changing summary
(someone might want to file a different bug on the plaintext quoting problem
since it's a different issue).
Hmm...the plain text editor problem may actually be an issue with HTML to plain 
text conversion. Thanks for helping me with this...I'll dig into the plain text 
issue.

- rhp
The problem is in CreateContextualFragment, which is throwing away most of the
html when it creates the fragment.  If I turn on the fragment dump just after
the CreateContextualFragment call, I see this:

============ Fragment dump :===========
###!!! ASSERTION: bad content: 'nsnull != mDocument', file nsGenericElement.cpp,
line 2039
###!!! Break: at file nsGenericElement.cpp, line 2039
@0x8b157b8 refcount=2<
###!!! ASSERTION: bad content: 'nsnull != mInner.mDocument', file
nsTextNode.cpp, line 210
###!!! Break: at file nsTextNode.cpp, line 210
  Text@0x8b3ff70
refcount=2<\n\u3053\u308c\u306fSJIS\u306e\u30da\u30fc\u30b8\u3067\u3059\u3002\n>
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
  br@0x8b4353c refcount=2<>
###!!! ASSERTION: bad content: 'nsnull != mInner.mDocument', file
nsTextNode.cpp, line 210
###!!! Break: at file nsTextNode.cpp, line 210
  Text@0x8b3ffb8 refcount=2<\u00a0\n>
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
  p@0x8b4000c refcount=2<
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
    a@0x8b40114 href=http://home.netscape.com/ja/ refcount=2<
###!!! ASSERTION: bad content: 'nsnull != mInner.mDocument', file
nsTextNode.cpp, line 210
###!!! Break: at file nsTextNode.cpp, line 210
      Text@0x8b40250 refcount=2<http://home.netscape.com/ja/>
    >
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
    base@0x8b402a4 href=http://home.netscape.com/ja/ refcount=2<>
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
    br@0x8b403e4 refcount=2<>
  >
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
  hr@0x8b40424 width=89% size=4 refcount=2<>
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
  br@0x8b404dc refcount=2<>
###!!! ASSERTION: bad content: 'nsnull != mInner.mDocument', file
nsTextNode.cpp, line 210
###!!! Break: at file nsTextNode.cpp, line 210
  Text@0x8b58830 refcount=2<\n\n>
###!!! ASSERTION: bad content: 'nsnull != mDocument', file
nsGenericHTMLElement.cpp, line 1147
###!!! Break: at file nsGenericHTMLElement.cpp, line 1147
  title@0x8b58884 refcount=2<
  >
>

which, if you remove all the asserts, translates to this:
@0x8b157b8 refcount=2<
  Text@0x8b3ff70
refcount=2<\n\u3053\u308c\u306fSJIS\u306e\u30da\u30fc\u30b8\u3067\u3059\u3002\n>
  br@0x8b4353c refcount=2<>
  Text@0x8b3ffb8 refcount=2<\u00a0\n>
  p@0x8b4000c refcount=2<
    a@0x8b40114 href=http://home.netscape.com/ja/ refcount=2<
      Text@0x8b40250 refcount=2<http://home.netscape.com/ja/>
    >
    base@0x8b402a4 href=http://home.netscape.com/ja/ refcount=2<>
    br@0x8b403e4 refcount=2<>
  >
  hr@0x8b40424 width=89% size=4 refcount=2<>
  br@0x8b404dc refcount=2<>
  Text@0x8b58830 refcount=2<\n\n>
  title@0x8b58884 refcount=2<
  >
>
This is definitely happening inside CreateContextualFragment -- it's getting
confused somehow (possibly by the doctype tag?), and creating a bad fragment
which is dumping out all the "bad content: 'nsnull != mInner.mDocument" asserts
when it's dumped.

Assigning to Rick -- can you take a look at this and at least give us a clue
what might be going wrong in the fragment content sink?  Cc'ing Vidur, and also
Harish, since he has some parser fixes in progress which are related to doctype
tags and might conceivably have something to do with this bug.
Assignee: akkana → rickg
OS: Windows NT → All
Hardware: PC → All
Blocks: 21856
Blocks: 18312
Akkana/rhp: I'll take a look, but I need a better testcase. Can you append the 
actual HTML fragment you're pasting, and then send this bug back to me? Thanks.
Assignee: rickg → akkana
Assigned Keyword beta1 and removed the wording from the Summary field.
Keywords: beta1
Summary: [BETA1] HTML InsertAsQuotation() seems to be cutting of HTML input → HTML InsertAsQuotation() seems to be cutting of HTML input
Back to rhp to post the fragment.  Maybe he can winnow it down to something
smaller.
Assignee: akkana → rhp
Putting on PDT+ radar for beta1.
Whiteboard: [PDT+]
Sorry for the complexity of the example, but this is the one that breaks the 
editor. I have others that work fine.

- rhp
Assignee: rhp → rickg
Rich -- I'm sending this back to you so that you can update the attachment. I 
can't get to it (it gives me an error when I click on the link). Please send it 
back once you've updated it.
Assignee: rickg → rhp
Rick,
You can do a right click -> "Save Link As" and save the data to bug.html. This 
should work.

- rhp
Assignee: rhp → rickg
I don't know why it's problematic yet, but here's the min test case. The culprit 
in the code is the <SCRIPT>:

<HTML>
<HEAD><SCRIPT LANGUAGE="JavaScript"><!-- --></SCRIPT></HEAD>
<BODY>Text here.</body>
</HTML>
Status: NEW → ASSIGNED
Well, well. It turns out that code was introduced in the parsing engine that 
"flushed" content when a script was encountered. That's logic confused the crap 
out of the fragmentContentSink(). I have a way to disable it when the editor is 
loading a page, but I'm looking for a better answer. Please hold...
Fixed by checkin last night.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
** Checked with 2/18/2000 Win32 build **

The test msg cotaining an HTML attachment in the Smoketest file (3rd
one) now quotes OK in Reply mode. I've tried other HTML attachments
and they were quoted OK.
The one problem I still see is that in forward/inline option,
I don't see an HTML attachment pulled into the Edit field and
instead I only see the main body of the original quoted.

Rich or Rick, should I file another bug for this problem?
OK. I corresponded with rhp about the attachment data not
gtting pulled into the Composer window though they actually
get included into the outgoing mail. That is how it was in 4.x and
it seems to be OK for Mozilla.

I've checked many msg types for quoting HTML attachments in
reply mode. Almost all I tried to seem to work OK -- i.e. attachments
of inlinetype get quoted in reply mode. 
There is one message which does ton get quoted. This is the 
3rd msg in the Intl Smoketest folder -- JA page at NetCenter
attached to a JPN msg. For some reason, this is the only message
which does not work so far. 
I'm inclined to open another bug for this unless Rich or someone
can debug the problem within the context of this bug.

Just choose the 3rd msg in the Smoketest folder and engage reply.
You will see only the main body quoted.
I'll wait until I hear from someone about this problem before
deciding what to do with the verification.
After corresponding with Rich,I've decided to re-open this bug for
the problem mentioned above for the 3rd msg in the Intl Smoketest.

This original message was composed in the following manner:

a. Went to http://home.netscape.com/ja using Comm 4.6.
b. Sent this page by "File | Send Page" under Japanese encoding.
c. S/MIME option was turned on.

I tried the same procedure as using Comm 4.72 and I have a nearly identical new message
to the above. Here's what is weird. This 2nd message has no problem with quoting
the inline attchment part in the reply mode. I played with the headers
for a while on these 2 msgs. I was able to get the original msg's attachment to 
get quoted uder reply mode by substituting RFC headers of the 1st msg with the
one from the 2nd msg, i.e. the header part before the Content-type multi-part line.
But then then 2nd msg (which was not touched) could not be quoted.

Anyway I attach these 2 msgs in one mailbox file to this bug report.
The message dated "4/27/99" is the original one causing the problem.
The message dated "2/22/2000" is the one which has no quoting problem.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Sending it to rhp.
Assignee: rickg → rhp
Status: REOPENED → NEW
This is still an insertion issue with the attached HTML.

- rhp
Assignee: rhp → rickg
Hey Rich: I don't mind looking at this bug, but please attach the plain HTML 
that is problematic to this so I can test more easily. Then send this bug back 
and I'll take look. Thanks.
Assignee: rickg → rhp
Hi Rick,
Here is the HTML data I'm trying to insert: 

   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=5720

- rhp
Assignee: rhp → rickg
PDT folks: I'm working on this issue with RHP, but as of yet I've not been able 
to reproduce it. We have a plan to try to narrow the search, and will retry on 
monday. 
Status: NEW → ASSIGNED
Let me update the status of this problem. With non-debug
builds this problem occurs on 2/27/2000 Win32 build still.
However, I saw the problem go away on 1 or 2 builds earlier in
the last several days. Sometimes, the same build may produce
the problem on one OS while it does not on another. But mostly
this problem has been occurring in the last 2-3 builds. 

I just wanted to note this erratic behavior so that it
may help debugging this problem.
Ok -- I'm getting nowhere. The attached HTML inserts fine for me, and I can't 
get the friggin' mail app to accept the test mail folder. So -- RHP, let's talk 
by phone on Thursday, and let's walk through the steps together; ok?
Sure...no problem. Just give me a call...if I'm not here, just leave a message 
and I'll get back to you.

- rhp
While looking Bug 29758, I found a workaround for this problem.
If you turn on ANY ONE of the Auto-Detect modules under the Browser's
View | Character Coding | ...., then you get the full quoting in the test 
message I uploaded.  What is surprising is that it does not have to be
Japanese Auto-Detection module to get this effect.
Please update the status whiteboard with a landing date.  There is some
discussion of a "work-around," and if that is viable, please consider it as a
way to get out of the way of the beta landing.
For now, I'm marking this to turn into a PDT- on 3/3.
Whiteboard: [PDT+] → [PDT+] w/b minus on 3/3
Harish and I discussed this bug yesterday. 
Our plan is as follows. I will scope the extent of
this remaining problem as of 3/1/2000. If the scope is limited,
then, we might work on this post Beta1 since we have a workaround.

I tried a number of pages tonight as attachments, and could not
find this exact problem. As of now, this problem is limited to
the example provided originally. Other attachments of similar 
nature get quoted fully in reply. Qouting should not fail
on this kind of page but my sense is that we can look at 
this post Beta1. The main purpose of this bug, i.e. quoting of
HTML attachments seem to be working generally. 

Comments suggest we're working well enough for beta1, and we passed the 3/3 
cutoff date... so I'm latering this to beta2
Whiteboard: [PDT+] w/b minus on 3/3 → [PDT-] plus for beta2
Target Milestone: M14 → M17
Putting on beta2 keyword radar since this was "plus for beta2" during beta1 
triaging
Keywords: beta2
Keywords: nsbeta2
harish, please work with rich pizarro on this one.
Assignee: rickg → harishd
Status: ASSIGNED → NEW
Keywords: beta2
** Re-tested with 5/4/2000 Win32 build **

The problem which occurs with Beta 1 or 3/24/2000 M15b1 build
no longer occurs with the above build on an exact set of
test cases. Whatever fixed it, I don't know but there is the
difference. 
So, I'm going to mark this bug worksfrome as of the above M16 build.
Status: NEW → RESOLVED
Closed: 25 years ago24 years ago
Resolution: --- → WORKSFORME
Verified as Worksforme.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: