23325 - Problems displaying msg with an inline attachment containing relative links.

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect, P3)

Description

[Katsuhiko Momoi]

** Observed with 1/6/2000 Win32 M13 build **

This crash started happening with the above build. My suspicion is
that it might be related to the new displaty code which got in
on the evening of 1/5/00. The problem is not reproducible with
1/5/00 Win32 build.

Here's a description of the problem:

1. Open Messenger.
2. Select a folder called 'jpattachtest' -- which will be attached to this
    bug report next.
3. Try to display the 2nd msg from the top. This is the one that crashes.
4. You don't have to have Japanese auto-detction turned on to get this
   crash.

This msg contains 2 body part -- JPN msg in iso-2022-jp in the first
part, then a JPN web page encoded in EUC.

The 'jpattachtest' file contains 4 msgs. In addition to Intl Smoketest
file, this one should be helpful in debugging display code modifications.
The file contains 4 JPN msgs with 4 types of attachments:

  1. JPN plain text main body part with JPN plain text attachment.
 *2. JPN HTML main body part with JPN EUC web page attachment.
  3. JPN HTML main body part with JPN ISO-2022-JP web page attachment.
  4. JPN HTML main body part with JPN Shifta_JIS web page attachment.

Msgs 2-4 pretty much share very similar MIME structure.
The asterisked one (2nd) is the one that crashes.

Comment 1

[Katsuhiko Momoi]

Both Naoki and I experinced this crash and both crashes produced
pretty much the same dump. Here's what it look likes:

=====================================================
Trigger Type:  Program Crash

Trigger Reason:  Access violation

Call Stack:    (Signature = nsImapUrl::GetImapMailFolderSink af9f45f6)

nsImapUrl::GetImapMailFolderSink
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapUrl.cpp, line 164]
nsImapProtocol::SetupSinkProxy
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapProtocol.cpp, line 467]
nsImapProtocol::LoadUrl
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapProtocol.cpp, line 1298]
nsImapIncomingServer::GetImapConnectionAndLoadUrl
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapIncomingServer.cpp, line
221]
nsImapMockChannel::AsyncRead
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapProtocol.cpp, line 6378]
nsDocumentOpenInfo::Open
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 211]
nsURILoader::OpenURIWithPostDataVia
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 499]
nsURILoader::OpenURIVia
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 456]
nsURILoader::OpenURI
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 442]
ImageNetContextImpl::GetURL
[d:\builds\seamonkey\mozilla\gfx\src\nsImageNetContextAsync.cpp, line 635]
IL_GetImage  [d:\builds\seamonkey\mozilla\modules\libimg\src\if.cpp, line 2070]
ImageRequestImpl::Init [d:\builds\seamonkey\mozilla\gfx\src\nsImageRequest.cpp,
line 261]
ImageGroupImpl::GetImage [d:\builds\seamonkey\mozilla\gfx\src\nsImageGroup.cpp,
line 274]
nsFrameImageLoader::Init
[d:\builds\seamonkey\mozilla\layout\base\src\nsFrameImageLoader.cpp, line 174]
nsPresContext::StartLoadImage
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp, line 832]
nsHTMLImageLoader::StartLoadImage
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLImageLoader.cpp, line
225]
nsHTMLImageLoader::GetDesiredSize
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLImageLoader.cpp, line
417]
nsImageFrame::GetDesiredSize
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsImageFrame.cpp, line 227]
nsImageFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsImageFrame.cpp, line 260]
nsLineLayout::ReflowFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsLineLayout.cpp, line 970]
nsBlockFrame::ReflowInlineFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3785]
nsBlockFrame::DoReflowInlineFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3674]
nsBlockFrame::DoReflowInlineFramesAuto
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3620]
nsBlockFrame::ReflowInlineFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3569]
nsBlockFrame::ReflowLine
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2753]
nsBlockFrame::ReflowDirtyLines
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2509]
nsBlockFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 1520]
nsBlockReflowContext::ReflowBlock
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockReflowContext.cpp, line
267]
nsBlockFrame::ReflowBlockFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3334]
nsBlockFrame::ReflowLine
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2702]
nsBlockFrame::ReflowDirtyLines
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2509]
nsBlockFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 1520]
nsAreaFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsAreaFrame.cpp, line 275]
nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line
644]
RootFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLFrame.cpp, line 334]
nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line
644]
nsScrollPortFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsScrollPortFrame.cpp, line
405]
nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line
644]
nsGfxScrollFrameInner::ReflowFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line
1254]
nsGfxScrollFrameInner::ReflowScrollArea
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line
1319]
nsGfxScrollFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line
521]
nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line
644]
ViewportFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp, line 527]
nsHTMLReflowCommand::Dispatch
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLReflowCommand.cpp, line
145]
PresShell::ProcessReflowCommands
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 1904]
PresShell::ExitReflowLock
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 828]
PresShell::ContentAppended
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 2393]
nsDocument::ContentAppended
[d:\builds\seamonkey\mozilla\layout\base\src\nsDocument.cpp, line 1546]
nsHTMLDocument::ContentAppended
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLDocument.cpp, line
1112]
HTMLContentSink::NotifyAppend
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLContentSink.cpp,
line 3721]
SinkContext::FlushTags
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLContentSink.cpp,
line 1804]
HTMLContentSink::CloseBody
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsHTMLContentSink.cpp,
line 2571]
CNavDTD::CloseBody [d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line
2674]
CNavDTD::CloseContainer [d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp,
line 2965]
CNavDTD::CloseContainersTo
[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 3025]
CNavDTD::CloseContainersTo
[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp, line 3207]
CNavDTD::DidBuildModel  [d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp,
line 628]
nsParser::DidBuildModel
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 591]

nsParser::ResumeParse [d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp,
line 984]
nsParser::OnStopRequest
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp, line 1376]
nsStreamConverter::OnStopRequest
[d:\builds\seamonkey\mozilla\mailnews\mime\src\nsStreamConverter.cpp, line 763]
nsDocumentOpenInfo::OnStopRequest
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp, line 243]
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]
=====================================================

Comment 2

[Katsuhiko Momoi]

Attachment: Mailbox with 4 JPN test msgs w/ JPN attachments

Comment 3

[Scott MacGregor]

Looking into this now.

Comment 4

[Scott MacGregor]

Kat, on my debug build from 01/09/00, I can display all of these japanese
messages (granted the fonts aren't right for me) but they all display without
crashing including the 2nd one.

Can you retest this on monday's build and tell me if it is still a problem for you?

Comment 5

[Scott MacGregor]

I found another message in my inbox that does cause a crash just like this. I
had seen it before but couldn't put my finger on what the pattern was. Now I see
what's going on. This crash happens on builds for me prior to the landing of my
new display stuff. I don't believe that's related.

I'm attaching another file that always produces the crash for me on imap.

Rich, the problem is that we are displaying an inline attachment that is an html
document. The document includes relative paths for things like images. i.e.
src="../images/buttons/editbtndown.gif"

Layout sees this relative url and it trys to resolve it against the document's
base url which is the imap url used to display the message: i.e.
"imap://mscott@dredd:143/fetch>UID>/INBOX>48556"
Trying to resolve images/buttons against the imap url is failing because that
operation is bogus. This leads to the crash.

Rich, shouldn't the mime parser be turning this into absolute links or should it
be setting a base url tag on the document so layout can properly resolve all the
relative urls in the document against the right base url (not the imap url that
is used to load the inline attachment but the actual url of the document).

Should I re-assign this to you?

Comment 6

[Scott MacGregor]

Attaching a mailbox folder containing an inline attachment with relative links (
a talk back link). I crash consistently with this message. Changing the subject
to reflect what is causing this crash.

Comment 7

[Scott MacGregor]

Attachment: msg that triggers the crash

Comment 8

[Katsuhiko Momoi]

Scott, I tried 1/10/00 Win32 build under Win98-J and I got the
same crash as I reported before. Talkback server is not working
right now and I can't send the report yet but it seems to be the same
crash.

ji reports that on Linux builds, we don't see this problem. I tried
1/6/00 Linux build myself, too, and it doesn't crashas ji says.

Comment 9

[Scott MacGregor]

Rich, can I assign this bug to you? I think mime either needs to insert a base
tag or something. I think in 4.5 we would whack the MWContext document URL
object in libmime to force it to be the url of the inlined attachment instead of
the message url. I wonder if we can use an HTML base tag or something instead
since we don't have a document url we can whack...

Comment 10

[rhp (gone)]

Just throw it my way...I'll take a look at it later today.

- rhp

Comment 11

[Scott MacGregor]

Ok, thanks Rich. Lemme know if you want to know more about the BASE url tag in
HTML. I think I had to use it for a school project once many years ago =).

Comment 12

[rhp (gone)]

Actually, I'm pretty familiar with BASE tags...until recently, Gecko didn't
support the tag on HTML fragments we inserted into a document which caused all
sorts of grief when you replied to a SendPage operation.

Hopefully, this will be an easy fix ;-)

- rhp

Comment 13

[rhp (gone)]

Good news! Vidur fixed this bug last week. The issue here was that Gecko used
to ignore BASE tags on fragment insertion. Now, Gecko honors it which rocks!
Now these messages all display without crashing.

Thank you Vidur!

- rhp



*** This bug has been marked as a duplicate of 21682 ***

Comment 14

[Katsuhiko Momoi]

This news tends to prove that the problem I reported is not attributable to "Base tag" issue.
The crash still occurs with yesterday's build (1/11/00 Win32) and the base tag bug was resolved
on 1/6/00. Or else, the other bug has not been resolved completely.
To dtermine, we need to look at the other bug, which I will do now.

Comment 15

[Scott MacGregor]

Rich, I saw this crash on builds well after the time vidur checked in his fix
for base tags. Re-opening. (I'm crashing on the example messages attached to
this bug report on the 1/11/00 build same as Kat.

Is mime emitting the base tag?

Comment 16

[Scott MacGregor]

I'd add that all my analysis showing how layout was resolving the relative urls
against the imap url used to load the document was done on a 1/9 debug build
which included Vidur's checkin on 1/6 (if that date is correct).

Comment 17

[rhp (gone)]

Ok, I see the problem now. This is an IMAP specific problem. The reason I
didn't see it was because I was looking at the message on a local mail folder.
mscott: I could probably use your help on this one.

- rhp

Comment 18

[Scott MacGregor]

Hey Rich, the problem isn't imap specific at all. It only happens to be that on
imap we crash, mailbox we don't. But even if you are using local mail, the
relative urls are incorrectly getting resolved against the mailbox url used to
load the message instead of the url for the inline attachment.

The problem exists for both types though. You should notice on local mail that
we just fail to download the resolved links where imap crashes.

If you want to see how I'm watching the urls get incorrectly resolved, set a
breakpoint in nsMsgMailNewsUrl::Resolve and then click on one of the messages
attached to this bug report.

Hope this helps.

Comment 19

[rhp (gone)]

Well, maybe I'm on drugs or something, but this messages DOES render fine when
stored in a local mail folder on my machine (images and all). I've traced this
through the debugger and for both cases (IMAP and mailbox) we do emit a BASE
tag. This is actually how 4.x worked as well.

Now I see what you are saying about the bad URL's being built in the
nsMsgMailNewsUrl::Resolve method, but in the mailbox case, the BASE tag has to
be getting picked up somewhere or I would never see all the correct images for
these inline pages.

- rhp

Comment 20

[Katsuhiko Momoi]

I concur with rhp that the problem msg does display OK if placed in a local mail folder.
I thought, however, that that was mscott's point -- that it would not crash locally.
My problem msg has only broken images -- I forgot to copy the image files on my server, so images
would not display anyway but the msg causes no crash.

Comment 21

[rhp (gone)]

Guys...when I say it displays OK, I don't mean that just doesn't crash...I mean
it displays perfectly...exactly as it does in 4.x. All remote images for the
web page are resolved and loaded...I see the images get loaded one by one. I'm
over a slow link so I can see the modem lights flash as it loads the images off
of the web site.

This is what I don't understand. Somewhere on my client, the BASE tag that I
write out from libmime is actually being used or I would never see images load.
I'm still investigating, but I do notice that when I load the message from the
IMAP server, the scheme (mScheme) in:

      nsStdURL::Resolve(const char *relativePath, char **result)

is wrong. It's "mailboxMessage:" even in the IMAP case. Still digging.

- rhp


- rhp

Comment 22

[Scott MacGregor]

That's funny. Are you sure it is in an imap folder? When I click on the message
in an imap folder the scheme = imap and not mailbox Message in ResolveScheme.

Comment 23

[Scott MacGregor]

The bug is in layout in nsHTMLImageElement::GetCallerSourceURL. This method
incorrectly determines the base url as the url in the webshell (which is the
imap url used to load the document). Instead, it should first try to figure out
if there is a base url already specified with a base tag and return that.

Fixing this method will fix the problem where we are incorrectly attempting to
resolve relative image urls against the url of the document despite the presence
of BASE tags in the document.

Adding rickg/vidur to the cc list in the hopes that he can help us find the
right owner for this as I think you mentioned that vidur did the work for
supporting the base url tag.

Comment 24

[Scott MacGregor]

I just talked to rickg about this bug and he said to go ahead and re-assign it
to vidur as he's the man for the base url stuff. vidur over to you...read the
comments I wrote just above this one to read how we are improperly resolving
urls for images against the document url instead of the url defined by the base
tag.

On a lighter note, I added some protection so we won't crash anymore. We just
need to get layout to stop trying to resolve the image urls against the
imap/mailbox url used to display the message =).

Comment 25

[vidur (gone)]

Actually, nsHTMLImageElement::GetCallerSourceURL() is only used when you
dynamically set the src of an image via script (and provide a relative URL). I
fixed a similar issue last week, but will look at it again tomorrow.

Comment 26

[vidur (gone)]

Couldn't get this done in time. It'll be first up for M14.

Comment 27

[vidur (gone)]

Hmm...the problem seems to be with the message itself. I'm guessing that the
source of the quoted page that's stored with the message is out of date - it's
referencing images that don't exist on the actual site. The resolution of
relative URLs is happening correctly - the images that the page references just
don't exist. I tried reading the same page sent via a Send Page and it worked
fine (both IMAP and local).

The original crash reported doesn't occur either. I'm marking this WORKSFORME.

Comment 28

[Katsuhiko Momoi]

I ceated the problem page intentially omitting the images from the image source directory.
There seem to have been two problems.

1. Msg display crashed on this kind of page lacking images referred to.
2. Relative links are resolved based on Base URL properly.

Apparently both have been fixed judging from mscott and vidur's recent comments.

My question at this point is on mscott's comment that he fixed the crashing problem --
Additional Comments From [e-mail]  2000-01-12 15:50 --

Scott, is this a permanent fix and will be be kept?

Comment 29

[Scott MacGregor]

Hey Vidur, I fixed the crash which is why you don't see the crash anymore. When
I transferred the bug to you, it was because HTMLImageelement still resolves
image urls against the document url instead of the base tag url. While it is
true that layout was never using these urls (it was resolving them again later
on against the correct base which is why the document rendered correctly).

My bug against you is because HTMLImageElement is incorrectly resolving these
relative urls against the document url by calling: GetCallerSourceURL
 and then calling MakeAbsoluteURL given that url as the base.

This is causing us to create bogus imap and mailbox urls which are never used
but nevertheless you are making us create them =). And in the case of imap urls,
these are heavy weight and construction can be expensive.

I hope this clarifies what I'm looking for. Let me know if it doesn't.

Comment 30

[leger]

Clearing WORKSFORME resolution due to reopen of this bug.  Taking "crash" out
of summary.

Comment 31

[vidur (gone)]

The problem that Scott is reporting has to do with SCRIPT in the head of the
attachment that does dynamic modification of the SCRIPT src property. The value
of the src property is set to a relative URL and has to be resolved (for
compatibility with 4.x) relative to the base URL of the calling script. From my
conversation with Scott, it may be that the BASE tag emitted by Rich isn't
emitted at the right spot i.e. after the HEAD content of the script.

Comment 32

[Katsuhiko Momoi]

scott, the crash is back! starting with yesterday's Win build. It is also reported on Linux.
It is the same message (2nd one in the atatchtest file). Has there been any late changes in this
area?

The crash needs to be fixed for M13. Accordingly changed TM to 13. Probably the crash part 
should go back to mscott.

Comment 33

[Scott MacGregor]

Yup, your example is crashing for me again now too. I just traced through it
again and it looks like either the base tag is not being emitted or the html
image loader isn't recognizing the base tag anymore.

I'm stepping through nsHTMLImageLoader::SetURL and the base URL is incorrectly
set to the imap url used to load the message and not the url in the base URL tag.

It definetly didn't used to be this way a few days ago.....

I think this belongs to rhp or vidur. The question is, is the base tag being
emitted or is it not being picked up in layout?

Comment 34

[Scott MacGregor]

I've traced through the mime code being generated and it is failing to emit a
BASE tag. Re-assigning to me instead of vidur as layout isn't seeing the base tag.

Rhp, can you help me look at this too? MimeInlineTextHTML_parse_begin fails to
emit a BASE tag for kat's message.

Comment 35

[rhp (gone)]

Sure...I'll look at this after dinner. :-)

- rhp

Comment 36

[Scott MacGregor]

I actually added PR_LOGGING to mime a couple weeks ago so I got a chance to put
it to good use. I'm going to attach the log showing what mime emits for Kat's
message. You can see that we don't have a base tag getting generated.

Now we need to just figure out why =)

Comment 37

[Scott MacGregor]

Attachment: PR Log of the html mime generates for this message.

Comment 38

[Scott MacGregor]

I have a small hack with should fix this problem and all other problems were we
try to illegally resolve a url against a imap or mailbox url. (I basically
return about:blank and an error code as the resolved url.

Getting reviews now...

Comment 39

[lchiang]

(Scott - how do we use this PR Logging?)

Comment 40

[Scott MacGregor]

Fix checked in. Lisa, you can use PRLogging on mime for seeing the html the mime
converter emits by doing the following:
In your favorite OS, set the following environment variables:
NSPR_LOG_FILE=nspr.log (or some other suitable file name of your choosing)
NSPR_LOG_MODULES=MIME:5

and presto! you'll see the html that mime is feeding layout...

Comment 41

[Katsuhiko Momoi]

** Checked with 1/24/00 Win32 build **

The crash no longer occurs with the above build. 
ji, please check also with today's Linux build and if no crash occurs in 
displaying the problem message, mark it verified/fixed.

Comment 42

[ji]

Verified with Linux 2000012401-M13 build. It is fixed.