Closed Bug 22655 Opened 25 years ago Closed 25 years ago

[DOGFOOD] Need to parse any quoted HTML and pass in charset for insert operation

Categories

(MailNews Core :: Composition, defect, P3)

x86
Windows NT
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: momoi, Assigned: rhp)

References

()

Details

(Whiteboard: [PDT-])

** Observed with 12/23/99 Win32 M13 build **

This bug was split from Bug 18312 since the phenomenon described
in the original crash bug no longer occurs and the 3rd type
of crash reported by nhotta and ducarroz in Bug 18312 seems to be
closer to what the original crash was about, i.e. failure
related to parsing some HTML elements.

Here's how to reproduce this bug with current M13 builds.

1. Turn on the Browser Japanes auto-detection by choosing
    View | Auto-Detect | Japanese.

2. Make sure that Mail send option is set to Plain Text.

3. Open Messenger and select the 3rd msg with attachment from
   the Intl Smoketest file found at the URL above.
4. Now engage Reply button.
5. Observe that while Mozilla is trying to quote materials into
   the Compose window, a crash occurs.

Bewlow are the relevant comments on this bug from Bug 18312:

===================================================================

------- Additional Comments From rhp@netscape.com  11/16/99 15:28 -------
Ok, I know what this specific bug is...its our old friend the META tag with a
charset specified. Basically, quoting HTML with this line will cause that
document reload and cause us to crash. I can prevent the data we generate from
having META tags (I've done that already) but there is no way to prevent
attachments from doing this.

Akkana: Is there a bug on this already...if so, its a dup, if not, then this is
a new one for Ender.

The stack trace looks like this:

nsDocLoaderImpl::GetContentViewerContainer(nsDocLoaderImpl * const 0x016dfa90,
unsigned int 1242164, nsIContentViewerContainer * * 0x0012e670) line 658 + 5
bytes
nsObserverBase::NotifyWebShell(nsObserverBase * const 0x016d8108, unsigned int
1242164, const char * 0x03c38f00, nsCharsetSource kCharsetFromMetaTag, const
char * 0x03c38e90) line 54 + 20 bytes
nsMetaCharsetObserver::Notify(nsMetaCharsetObserver * const 0x016d8100,
unsigned int 1242164, unsigned int 5, const unsigned short * * 0x0012ed34,
const unsigned short * * 0x0012edfc) line 287 + 36 bytes
nsMetaCharsetObserver::Notify(nsMetaCharsetObserver * const 0x016d8100,
unsigned int 1242164, const unsigned short * 0x0012ecb4, unsigned int 5, const
unsigned short * * 0x0012ed34, const unsigned short * * 0x0012edfc) line 166
nsObserverNotifier::operator()(void * 0x016d8100) line 321 + 47 bytes
nsDeque::FirstThat(nsDequeFunctor & {...}) line 348 + 14 bytes
CObserverService::Notify(nsHTMLTag eHTMLTag_meta, nsIParserNode & {...},
unsigned int 1242164, const char * 0x02194094, nsIParser * 0x03c17b60) line 938
CNavDTD::WillHandleStartTag(CToken * 0x02298630, nsHTMLTag eHTMLTag_meta,
nsCParserNode & {...}) line 1065 + 35 bytes
CNavDTD::HandleStartToken(CToken * 0x02298630) line 1258 + 20 bytes
CNavDTD::HandleToken(CNavDTD * const 0x03c26ba0, CToken * 0x02298630, nsIParser
* 0x03c17b60) line 732 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x03c26ba0, nsIParser * 0x03c17b60,
nsITokenizer * 0x03c34d40, nsITokenObserver * 0x00000000, nsIContentSink *
0x03c218f0) line 529 + 20 bytes
nsParser::BuildModel() line 1030 + 34 bytes
nsParser::ResumeParse(nsIDTD * 0x00000000, int 0) line 956 + 11 bytes
nsParser::Parse(const nsString & {""}, void * 0x0012f434, const nsString &
{"text/html"}, int 0, int 1, eParseMode eParseMode_autodetect) line 837 + 15
bytes
nsParser::ParseFragment(const nsString & {"<!doctype html public "-//w3c//dtd
html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}, void * 0x00000000,
nsITagStack & {...}, unsigned int 0, const nsString & {"text/html"}, eParseMode
eParseMode_autodetect) line 926 + 41 bytes
nsRange::CreateContextualFragment(nsRange * const 0x03c17884, const nsString &
{"<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}, nsIDOMDocumentFragment * *
0x0012f7f4) line 1845 + 52 bytes
nsHTMLEditor::InsertHTML(nsHTMLEditor * const 0x035fc5a4, const nsString &
{"<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}) line 1358 + 63 bytes
nsHTMLEditor::InsertAsCitedQuotation(nsHTMLEditor * const 0x035fc5a8, const
nsString & {"<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}, const nsString & {""}) line
3653 + 20 bytes
nsHTMLEditorLog::InsertAsCitedQuotation(nsHTMLEditorLog * const 0x035fc5a8,
const nsString & {"<!doctype html public "-//w3c//dtd html 4.0
transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}, const nsString & {""}) line
457 + 17 bytes
nsHTMLEditor::InsertAsQuotation(nsHTMLEditor * const 0x035fc5a8, const nsString
& {"<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}) line 3575 + 23 bytes
nsHTMLEditorLog::InsertAsQuotation(nsHTMLEditorLog * const 0x035fc5a8, const
nsString & {"<!doctype html public "-//w3c//dtd html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}) line 421 + 13 bytes
nsEditorShell::InsertAsQuotation(nsEditorShell * const 0x03c26f30, const
unsigned short * 0x036d2d80) line 2135 + 42 bytes
nsMsgCompose::ConvertAndLoadComposeWindow(nsIEditorShell * 0x03c26f30, nsString
{"<br><br>Katsuhiko Momoi wrote:<br><html>"}, nsString {"<!doctype html public
"-//w3c//dtd html 4.0 transitional//en">
<html>
&nbsp;
<br><A HREF="http://people.netscape.com/momoi/"}, nsString ...) line 140
QuotingOutputStreamListener::OnStopRequest(QuotingOutputStreamListener * const
0x03c34bb0, nsIChannel * 0x03b14090, nsIChannel * 0x03b14090, unsigned int 0,
nsIChannel * 0x03b14090) line 970
nsStreamConverter::OnStopRequest(nsStreamConverter * const 0x03b11d70,
nsIChannel * 0x03b10264, nsISupports * 0x03be46f0, unsigned int 0, const
unsigned short * 0x00000000) line 730
nsMsgProtocol::OnStopRequest(nsMsgProtocol * const 0x03b10260, nsIChannel *
0x03b22a50, nsISupports * 0x03be46f0, unsigned int 0, const unsigned short *
0x00000000) line 199 + 74 bytes
nsMailboxProtocol::OnStopRequest(nsMailboxProtocol * const 0x03b10260,
nsIChannel * 0x03b22a50, nsISupports * 0x03be46f0, unsigned int 0, const
unsigned short * 0x00000000) line 177
nsFileChannel::OnStopRequest(nsFileChannel * const 0x03b22a54, nsIChannel *
0x03b248b0, nsISupports * 0x03be46f0, unsigned int 0, const unsigned short *
0x00000000) line 427 + 45 bytes
nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x03b2c850) line
326
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x03b29e70) line 173 + 12 bytes
PL_HandleEvent(PLEvent * 0x03b29e70) line 537 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x01082c20) line 498 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00870b12, unsigned int 49389, unsigned int 0,
long 17312800) line 972 + 9 bytes
USER32! 77e71268()
01082c20()


------- Additional Comments From akkana@netscape.com  11/16/99 15:50 -------
Adding Rick and Vidur to cc list, since this is happening when ParseFragment is
called on a fragment which contains a meta charset tag, which incorrectly
triggers a reload.

Perhaps the parser should ignore meta charset tags when parsing fragments -- or
will that cause problems for people who want to be able to paste html with a
different charset than the one currently in the document?

------- Additional Comments From rhp@netscape.com  11/18/99 22:32 -------
Akkana: who do you think this should be assigned to...its a PDT+ bug and the
fix lives in layout.

- rhp

------- Additional Comments From akkana@netscape.com  11/19/99 11:39 -------
I really don't know -- I've sent mail to Rick, Vidur and Ftang asking them to
take a look at the bug and venture an opinion.

------- Additional Comments From rhp@netscape.com  11/22/99 16:55 -------
Hi Rick,
Since we have a plan for this bug, I figured I would update it and pass it over
to you. Then, when you are done, you can pass it to Akkana for her API updates
and finally back to me for the insert call changes.

After you fix the crash, we should probably take it off of the PDT radar.

Thanks for the help!

- rhp

Here is the plan of attack:


Here's the problem as far as I can tell:

Rich wants to insert an HTML fragment into an existing document for the purpose
of block quoting. The fragment he's inserting contains
a <meta> tag with a charset specifier. (Note: The parsing engine has observers
for tags, and the i18n library ties into this notification
system for the purpose of charset detection). When ParseFragment() is called,
the <meta> tag is observed by the i18n library, which
detects the charset specifier. As a result, the i18n library tells the parser
to stop loading the document, and then it tells the webshell to
reload the document using a different charset.
Since this was a fragment and not a document, we get a crash, though I didn't
check into why we are crashing exactly.

The solution: First, we'll disable <meta> observation on HTML fragments.
Second, we need to extend the ParseFragment() api to
include a charset specifier. Anyone who wants to call this method will need to
determine the charset of the fragment in advance. Frank
Tang has a few i18n library calls to do this: nsIPlatformCharset() and
nsICharsetDetector().

I'll be out the rest of this week, but I'll add a quick hack tonight to disable
notification when ParseFragment() is called. When I return, I'll
extend the API to require a charset specifier. Akkana will need to update her
calls to ParseFragment() to include a charset.


------- Additional Comments From bobj@netscape.com  11/23/99 12:42 -------
I don't know what an HTML fragment really is, but should it be including
<HEAD> info?  <META> tags are only legal in the HTML head and not in the body.

------- Additional Comments From momoi@netscape.com  1999-12-06 15:52 -------
Apparently the reason that the crash does not occur when the auto-detection is
turned off
is that quoting of Japanese attachment does not occur in that case -- this could
be another bug.

------- Additional Comments From bobj@netscape.com  1999-12-07 14:55 -------
Is this the same bug?
The Swedish page mentioned above (1) does not have any <META> tags, and
(2)it uses <LAYER> tags...

<LAYER> tags are not supported right now, but they shouldn't shouldn't
cause a crash (if that ***is*** the problem).

------- Additional Comments From ji@netscape.com  1999-12-14 17:31 -------
With linux 121411-M12 build, the crash still happens when replying in plain text
mode with charset auto detector turned on. When the charset auto detector is
turned off, the crash doesn't happen either in plain text or HTML mode.

------- Additional Comments From momoi@netscape.com  1999-12-14 18:03 -------
On 12/14/99 Win32 M12 build (1999121408), I can also reproduce
this problem under plain text send option only  with the Auto-Detect
(Japanese) turned on. When it is off, the attachment does not
get quoted and so apparently avoids this crash.

------- Additional Comments From bobj@netscape.com  1999-12-15 11:18 -------
Do you (ji and momoi) have stack traces for your crashes?  Or, are they the
same as the ones already in the bug report?

Is crashing on quote replies with Japanese auto-detect on, a dogfood problem?
Seems like this is the normal state for users reading/writing Japanese email.
This was demoted when we thought the crashing was fixed, if we need to
reconsider, please clear "[PDT-]" from the Whiteboard and explain why.

Also, I'm confused by the symptom and the analyis. Why would whether Japanese
auto-detect is on or off, affect the pasting of a fragment?  No matter what
was detected, aren't we always going to include a META tag in the pasted
fragment?

------- Additional Comments From momoi@netscape.com  1999-12-15 11:35 -------
Here's the dump I got today with 12/14/99 Win32 build using the
steps I described above. Does not look the same as the one before.

-------------------------
Trigger Type:  Program Crash

 Trigger Reason:  Access violation

 Call Stack:    (Signature = nsDocLoaderImpl::GetContentViewerContainer
eec3f3d3)

   nsDocLoaderImpl::GetContentViewerContainer

[d:\builds\seamonkey\mozilla\webshell\src\nsDocLoader.cpp, line 810]

   nsObserverBase::NotifyWebShell

[d:\builds\seamonkey\mozilla\intl\chardet\src\nsObserverBase.cpp, line 60]

   nsMetaCharsetObserver::Notify

[d:\builds\seamonkey\mozilla\intl\chardet\src\nsMetaCharsetObserver.cpp, line
267]

   nsMetaCharsetObserver::Notify

[d:\builds\seamonkey\mozilla\intl\chardet\src\nsMetaCharsetObserver.cpp, line
145]

   nsObserverNotifier::operator()

[d:\builds\seamonkey\mozilla\htmlparser\src\nsDTDUtils.h, line 322]

   nsDeque::FirstThat

[d:\builds\seamonkey\mozilla\xpcom\ds\nsDeque.cpp, line 365]

   CObserverService::Notify

[d:\builds\seamonkey\mozilla\htmlparser\src\nsDTDUtils.cpp, line 928]

   CNavDTD::WillHandleStartTag

[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 1101]

   CNavDTD::HandleStartToken

[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 1286]

   CNavDTD::HandleToken

[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 764]

   CNavDTD::BuildModel

[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 528]

   nsParser::BuildModel

[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 1044]

   nsParser::ResumeParse

[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 968]

   nsParser::Parse

[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 845]

   ConvertBufToPlainText

[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgCompUtils.cpp, line 1952]

   QuotingOutputStreamListener::ConvertToPlainText

[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgCompose.cpp, line 1049]

   QuotingOutputStreamListener::OnStopRequest

[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgCompose.cpp, line 1161]

   nsStreamConverter::OnStopRequest

[d:\builds\seamonkey\mozilla\mailnews\mime\src\nsStreamConverter.cpp, line 736]

   nsOnStopRequestEvent::HandleEvent

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp, line
279]

   nsStreamListenerEvent::HandlePLEvent

[d:\builds\seamonkey\mozilla\netwerk\base\src\nsAsyncStreamListener.cpp, line
94]

   PL_HandleEvent
                                                  [plevent.c, line 523]

   _md_EventReceiverProc
                                                  [plevent.c, line 951]

   USER32.dll + 0x1186 (0x77e41186)

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

------- Additional Comments From momoi@netscape.com  1999-12-15 11:39 -------
The crashes we are seeing recently may not be related to
the original bug I reported. If so, we probably should create
another bug for this latetr problem.

------- Additional Comments From bobj@netscape.com  1999-12-15 11:49 -------
Create a new bug please.


------- Additional Comments From rhp@netscape.com  1999-12-15 11:52 -------
Eventually, we will scan the data to insert for the charset and pass that in to
the insert call. No META tags.

- rhp

------- Additional Comments From phil@netscape.com  1999-12-20 10:18 -------
The crashing part of this bug seems like a beta stopper.


====================== End of quote =========================================
QA Contact: lchiang → momoi
Assigning myself as QA Contact.
The most telling comment I saw was a request from bobj to "create a new bug"
since this bug seems to have migrated quite a bit over time.  Please do so, and
then we'll try to figure out (if you want to nominate it as dogfood) if it
should be PDT+/-
Thanks,
Jim
jar, this bug was created in response to Bobj's comment you refer to.
So this is in fact that bug. Most of the comments in this bug
have been quoted from Bug 18312 except the preface I wrote and
another comment I wrote after == end of the quote ==.

This crash bug is a very serious one affecting the reply/quoting of
an atatchment which contains a meta charset tag under Plain text mail.
Status: NEW → ASSIGNED
Target Milestone: M13
Have a beat on this one.

- rhp
Putting on PDT- radar for now.

rhp - please contact rickg for help with the parser issue.  Will re-evaluate if
a PDT+ or PDT-.
Whiteboard: [PDT-]
Putting on PDT- radar as indicated in Jan's comment above (but the annotation
was not added at the time).
I have a possible fix for this being reviewed!

- rhp
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Ok, this bug should be fixed and will now give you better rendering on HTML
attachments with META tags that define charset's.

Now, I am going to open up a different bug for the quoting getting cutoff when
being inserted into the document. I have a feeling that it is a META tag issue
and I may be able to address it in libmime, but let me experiment first.

- rhp
Blocks: 24206
** Checked with 1/24/00 Win32 build **

The crash no longer occurs with the test case I mentioned above.
However, it is also true that inline HTML attachment does not get quoted
in the reply mode. It seems to be quoting only the 1st body part. Makes reply/quote
a lot less interesting at this point. 
Let's hope that Bug 23931 gets resolved soon so that we can see the inline material
quoted.

ji, please chaeck the results for Linux and mark it verified if you don't see the problem, either.
Checked with linux 2000012401-M13 build. Same result as Win32 build.
No crash occurs when replying to the test mail.
Status: RESOLVED → VERIFIED
No longer blocks: 24206
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.