Closed Bug 81576 Opened 23 years ago Closed 23 years ago

when this web page loads it brings up a "save as" dialog.

Categories

(SeaMonkey :: UI Design, defect, P1)

Product:
SeaMonkey
Component:
UI Design
Platform:
x86
All
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: skasinathan, Assigned: nisheeth_mozilla)

References

(
URL
)

Details

(Keywords: crash, Whiteboard: MUST HAVE;)

Attachments

(8 files)

screenshot attached. Make it "5" save as per minute.
208.21 KB, image/jpeg
Details
Don't load links that aren't within head
2.12 KB, patch
Details | Diff | Splinter Review
Don't load links that aren't within head v2
2.95 KB, patch
Details | Diff | Splinter Review
Don't load links that aren't within head v3
3.41 KB, patch
Details | Diff | Splinter Review
Screenshot of mtv.com in Netscape build with patch
50.19 KB, application/octet-stream
Details
ie50.zip - mtv.com screenshot in IE 5.0
51.99 KB, application/zip
Details
Patch to fix popping up of multiple save as dialogs.
543 bytes, patch
Details | Diff | Splinter Review
screen shot for cnn.com attached
19.53 KB, image/jpeg
Details 
I've no clue why this brings up mutiple "Save As" dialog once this web page is
loaded. Very annonying :)

Build: Todays commercial build on Win NT.
Keywords: nsbeta1, nsdogfood
->bill
Assignee: pchen → law
nav triage: this is an rtm stopper esp since marketwatch is a top100 type of 
site and we dont know if this bug shows up elsewhere as well.
Keywords: nsbeta1, nsdogfood → nsbeta1+, nsdogfood-
Priority: -- → P1
Target Milestone: --- → mozilla0.9.2
I see the same thing on RedHat Linux 7.1 using build id 2001051815

More specifically, I see 4 "Downloading" dialogs, all with the type text/html
(isn't there already a bug about this?) and doubleclick URLs.
OS: Windows NT → All
Another URL where this happens - http://www.msnbc.com/news/575492.asp?cp1=1
why is this bug for 0.9.2. Why not for 0.9.1? matketwatch.com is one of the top 
pages and I'm pretty sure _most_ of the people visit this page very frequently. 
How do I nominate this for 0.9.1?
Keywords: mozilla0.9.1
Whiteboard: 0.9.1
*** Bug 81890 has been marked as a duplicate of this bug. ***
Are people still seeing this in recent builds?  There was a networking checkin 
on May 18th that was supposed to fix this for HTTP/0.9 content...
I am seeing this problem at
http://www.tehelka.com/currentaffairs/may2001/ca052101bjp.htm

Mozilla trunk build 2001052204 (today's) on Win2000.
Build Id 2001052210, RedHat Linux 7.1 - still see it on http://www.marketwatch.com
Can't reproduce the bug on Win32 platform using http://www.marketwatch.com
But same sort of things DO happen on
http://www.us.buy.com/retail/computers/department.asp?loc=101
Can only reproduce for
http://www.tehelka.com/currentaffairs/may2001/ca052101bjp.htm with a current
cvs build on linux. Problem here (whitespace after "Content-Type: text/html"
will be fixed by the patch attached to bug 80313.
I still see this problem for the original url (cbsmarketwatch.com) I reported.
It is very very annoying. I see 4 Save as dialog. I _think_ that page has auto
refresh for every one minute (just like most of the web sites). So it brings up
the 4 Save as dialog for every minute the user is in that page.  

Is it just me seeing these kinda weird things happening on our product?? 

 
Just got 5 dialogs on marketwatch, build 2001052819 on RedHat Linux 7.1
Ok. I see this again on few other sites while I was navigating tgh my history. 
Can this be looked into for 0.9.1? (asking again). Thanks!
Reloaded http://www.marketwatch.com several times and got a random number of 0..4
download dialogs per reload. I'm pretty sure that some ad servers send unusal
Content-Type headers which we are treating wrong.
Adding dependency on bug 80313 (patch approved for 0.9.1).
Depends on: 80313
Hmm, I'm not as sure as before. Behavior at http://www.marketwatch.com is
different from http://www.tehelka.com/currentaffairs/may2001/ca052101bjp.htm :
Mozilla knows a long name for "text/html", i.e. it seems to recognize the
correct MIME type.

the patch for 80313 doesn't seem to fix this one.
in fact i can't seem to locate an instance of an improperly parsed content-type
header while loading http://www.marketwatch.com/  there must be some other
explanation for this popup.
this bug does NOT depend on bug 80313.
No longer depends on: 80313
adding this back to mozilla0.9.1 milestone. This is ugly and needs to be fixed.
Target Milestone: mozilla0.9.2 → mozilla0.9.1
Whiteboard: 0.9.1 → no eta yet
Using a debug build, I get an assertion at line 100 in 
nsDSURIContentListener.cpp (that's the "doc shell URI content listener").  It's 
inside ::DoContent and happens because mDocShell is null.  NS_ERROR_FAILURE is 
returned and the caller, nsDocumentOpenInfo::DispatchContent, thinking, 
rightfully so, that no content listener has been found, then hands the request 
off to the nsExternaHelperAppService, which shows the helper app dialog.

A similar thing happens for 3 other uri's within that page.

The request in question is for the url: 
http://ad.doubleclick.net/adi/news.cbsmw.com/frontpage;sc=frontpage;ptile=1;sz=3
92x72;u=392x72;ord=197551906?".  The content type is "text/html."

It appears that this should return the normal stream listener like this 
function does for all other text/html.  It can't in this case, apparently 
because mDocShell is null.

mDocShell is set here: 
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#5677.  The 
only way that call can be bypassed is if the previous call to 
nsDSURIContentListener::Init fails.

That function will fail only if the the do_GetService for the category manager 
fails here: 
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDSURIContentListener.cpp
#46.

I don't think that is failing (mCatMgr is not null).

So I'm stumped.  I'm going to set a watch breakpoint on mDocShell and see if I 
can figure out what's going on.
OK, mDocShell is null because the doc shell is being destroyed (and 
nsDocShell::Destroy calls nsDSURIContentListener::DocShell(0)).

Why/how that is is beyond me at this point.  Here's the call stack at the point 
where this happens:

nsDSURIContentListener::DocShell(nsDocShell * 0x00000000) line 233
nsDocShell::Destroy(nsDocShell * const 0x03b5903c) line 2392
nsWebShell::Destroy(nsWebShell * const 0x03b5903c) line 1424
nsHTMLFrameInnerFrame::~nsHTMLFrameInnerFrame() line 592
nsHTMLFrameInnerFrame::`scalar deleting destructor'(unsigned int 0x00000001) + 
15 bytes
nsFrame::Destroy(nsFrame * const 0x03d07154, nsIPresContext * 0x03375788) line 
428 + 34 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0381c0d8, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x03b6b798, nsIPresContext * 
0x03375788) line 119
nsLineBox::DeleteLineList(nsIPresContext * 0x03375788, nsLineBox * 0x03ba83ec) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x03b6bee0, nsIPresContext * 
0x03375788) line 313 + 16 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x03b6be84, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x03b6be3c, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x03b6b66c, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x03b6b604, nsIPresContext * 
0x03375788) line 119
nsTableFrame::Destroy(nsTableFrame * const 0x03b6b604, nsIPresContext * 
0x03375788) line 278
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0380a3bc, nsIPresContext * 
0x03375788) line 119
nsTableOuterFrame::Destroy(nsTableOuterFrame * const 0x0380a3bc, nsIPresContext 
* 0x03375788) line 69
nsLineBox::DeleteLineList(nsIPresContext * 0x03375788, nsLineBox * 0x03ba848c) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x0381c480, nsIPresContext * 
0x03375788) line 313 + 16 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0380a360, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0380a318, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0380a4fc, nsIPresContext * 
0x03375788) line 119
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0380a494, nsIPresContext * 
0x03375788) line 119
nsTableFrame::Destroy(nsTableFrame * const 0x0380a494, nsIPresContext * 
0x03375788) line 278
nsFrameList::DestroyFrames(nsIPresContext * 0x03375788) line 116
nsContainerFrame::Destroy(nsContainerFrame * const 0x0380a7a4, nsIPresContext * 
0x03375788) line 119
nsTableOuterFrame::Destroy(nsTableOuterFrame * const 0x0380a7a4, nsIPresContext 
* 0x03375788) line 69
nsLineBox::DeleteLineList(nsIPresContext * 0x03375788, nsLineBox * 0x03ba857c) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x0380a00c, nsIPresContext * 
0x03375788) line 313 + 16 bytes
nsLineBox::DeleteLineList(nsIPresContext * 0x03375788, nsLineBox * 0x03ba8644) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x03809f40, nsIPresContext * 
0x03375788) line 313 + 16 bytes
nsFrameList::DestroyFrame(nsIPresContext * 0x03375788, nsIFrame * 0x03809f40) 
line 202
CanvasFrame::RemoveFrame(CanvasFrame * const 0x03817104, nsIPresContext * 
0x03375788, nsIPresShell & {...}, nsIAtom * 0x00000000, nsIFrame * 0x03809f40) 
line 367
FrameManager::RemoveFrame(FrameManager * const 0x037924f8, nsIPresContext * 
0x03375788, nsIPresShell & {...}, nsIFrame * 0x03817104, nsIAtom * 0x00000000, 
nsIFrame * 0x03809f40) line 882
nsCSSFrameConstructor::ReconstructDocElementHierarchy(nsCSSFrameConstructor * 
const 0x03624690, nsIPresContext * 0x03375788) line 7427 + 61 bytes
StyleSetImpl::ReconstructDocElementHierarchy(StyleSetImpl * const 0x03651458, 
nsIPresContext * 0x03375788) line 1225
PresShell::ReconstructFrames() line 4913 + 38 bytes
PresShell::StyleSheetAdded(PresShell * const 0x03764490, nsIDocument * 
0x037ef0d8, nsIStyleSheet * 0x03c00660) line 4925
nsDocument::InsertStyleSheetAt(nsDocument * const 0x037ef0d8, nsIStyleSheet * 
0x03c00660, int 0x00000004, int 0x00000001) line 1313
CSSLoaderImpl::InsertSheetInDoc(nsICSSStyleSheet * 0x03c00660, int 0x00000007, 
nsIContent * 0x03d0c500, int 0x00000001, nsICSSLoaderObserver * 0x03d0c534) 
line 1099
CSSLoaderImpl::SheetComplete(nsICSSStyleSheet * 0x03c00660, SheetLoadData * 
0x03d0c988) line 809
CSSLoaderImpl::ParseSheet(nsIUnicharInputStream * 0x033f0ba0, SheetLoadData * 
0x03d0c988, int & 0x00000001, nsICSSStyleSheet * & 0x03c00660) line 864
CSSLoaderImpl::DidLoadStyle(nsIStreamLoader * 0x03d0cde0, nsString * 
0x03666600, SheetLoadData * 0x03d0c988, unsigned int 0x00000000) line 899 + 27 
bytes
SheetLoadData::OnStreamComplete(SheetLoadData * const 0x03d0c988, 
nsIStreamLoader * 0x03d0cde0, nsISupports * 0x00000000, unsigned int 
0x00000000, unsigned int 0x0000080b, const char * 0x03d574c8) line 651
nsStreamLoader::OnStopRequest(nsStreamLoader * const 0x03d0cde4, nsIRequest * 
0x03d0cea0, nsISupports * 0x00000000, unsigned int 0x00000000) line 120 + 81 
bytes
nsStreamListenerTee::OnStopRequest(nsStreamListenerTee * const 0x03cfe488, 
nsIRequest * 0x03d0cea0, nsISupports * 0x00000000, unsigned int 0x00000000) 
line 25
nsHttpChannel::OnStopRequest(nsHttpChannel * const 0x03d0cea4, nsIRequest * 
0x03d0db50, nsISupports * 0x00000000, unsigned int 0x00000000) line 2056
nsOnStopRequestEvent::HandleEvent() line 159
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x0328f0f4) line 64
PL_HandleEvent(PLEvent * 0x0328f0f4) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00c38d80) line 520 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x009104be, unsigned int 0x0000c11c, unsigned 
int 0x00000000, long 0x00c38d80) line 1071 + 9 bytes
USER32! 77e13eb0()
USER32! 77e1401a()
USER32! 77e192da()
nsAppShellService::Run(nsAppShellService * const 0x00d07de0) line 418
main1(int 0x00000001, char * * 0x00358738, nsISupports * 0x00000000) line 1128 
+ 32 bytes
main(int 0x00000001, char * * 0x00358738) line 1426 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

This bug is being caused by a link element inside the body of the Web page.

<link rel="stylesheet" type="text/css" href="/styles/css/phat.css" />

A link element is only allowed in the head of a document, so this is invalid 
HTML.  Even so, we should be robust enough to ignore the link element in the 
content sink code when we don't find it in the HEAD.

By not ignoring it, we're doing a full frame reconstruct on this page, which is 
incredibly slow!  We might want to break out a separate evangelism bug and get 
them to change the page, but in the meantime, we should patch the content sink 
to ignore link elements that don't occur in the head of the doc.

Thanks, Dave.  To clarify a bit further (for my own benefit), the problem is 
that we do process that <link> element, and do so by destroying the previously 
constructed <iframe>(s).  Some of those have pending network requests and when 
those come in, we hit the assertion (and subsequent helper app dialog).

I'd like to re-assign this to somebody who can tweak the handling of <link> 
elements (and with time to work on this).  Dave suggests Peter V (who I'm 
adding to the cc: list).  Peter, can you take this, please?
There's more to it than just the link element. After I fixed the content sink to
not load the stylesheet, I still get two "Save As" dialogs. I'll attach my
patch. The other dialogs seem to be coming from the iframes with src pointing to
pages at http://ad.doubleclick.net/adi/news.cbsmw.com/. Spamming some people who
could help with this (if they have the time).
I spoke too soon I think, the problem does seem gone with this patch. We
probably need a similar fix in the XML content sink.
The patch doesn't fix it for me, it seems.  I stopped in ProcessLINKTag with the
debugger and verified that mCurrentContext==mHeadContext every time it was
called.  Yet I still have the doc shell destroyed out from under the
nsDSURIContentListener (which then triggers the helper app dialog).

There must be more to it (or else I'm screwed up).  Anybody else care to test
this patch?

Peter, should you take this bug, or should I find somebody else?
I still see the problem too. This page's source is so fucked up.
I need help from parser people for this one. Harish: I wonder why my check for
mCurrentContext == mHeadContext is always true while loading this page. The html
is pretty bad, but they do have a closing head tag before the second link
element. OTOH, doing <html><body><head> is asking for trouble imo. Guess I'll
take this for now :(.
Assignee: law → peterv
peter: mCurrentContext == mHeadContext is always true due to ( I'm guessing
here! ) the lack of </HEAD>. Theoretically, the context switch should happen
based on the content that is being dealt with. This sounds like a bug in the parser.
Oops sorry. I do see the /HEAD ( spoke too soon ).
LINK tag's parent model is HEAD. That is the parser would trigger context switch  
( to headcontext ), in the sink, when it encounters a LINK tag. This the reason 
why mCurrentContext == mHeadContext is always true in processing link tags. That 
is, the parser & the sink are behaving correctly.
Peter: May be you should use the memeber "mBody" which gets set when the BODY 
gets processed. That is something like 

-      if (ssle) {
+      // Only enable loading of the stylesheet for link elements that are
+      // in the right spot (within the head element).
+      if (ssle && !mBody) {
         // XXX need prefs. check here.
         ......
QA Contact: sairuh → bsharma
I tried modifying the patch to check !mBody.

This gets me a couple of assertions that "LINK element outside head" but I 
still get the assertions in nsDSURIContentListener and the docShell has still 
been removed out from under the pending load requests.
Here's my theory (for what it's worth):

Did we change recently to load style-sheets "asynchronously" (i.e., while 
blocking the parser/content-sink).  I think maybe Dave Hyatt mentioned that 
when I was talking to him about this bug.  If that's the case, then maybe the 
problem is that the <iframe>s are being destroyed even for <link> elements 
inside the <head>.  That's presuming they would get destroyed when the style-
sheet finished loading.
Summary: when this web page loads it brings up a "save as" dialog. → MUST HAVE; when this web page loads it brings up a "save as" dialog.
Summary: MUST HAVE; when this web page loads it brings up a "save as" dialog. → when this web page loads it brings up a "save as" dialog.
Whiteboard: no eta yet → MUST HAVE; no eta yet
This is happening at http://www.cnn.com as well.  It seems to happen only when 
I return to the main page (after reading one of the articles, for example).
I just noticed that this bug was filed on May 17th. My stylesheet loading
changes landed on May 18th in the evening. I'm looking through other changes
from before the 17th that might have caused this.
+mostfreq
Isn't this and Bug 82236 the same?
Keywords: mostfreq
*** Bug 84040 has been marked as a duplicate of this bug. ***
Is there any "avoid the crash" hack we can put in m.9.1?  We're almost
completely out of time...
With this latest patch, I'm not seeing the dialogs anymore. Could someone else
please try it out and let me know if it works?
woha. This patch works fine for me
Bill, you tried a similar patch before and it didn't help, could you try this
one? Thanks.
Heikki, jst, could you please (super-)review just in case it does work and we
want to get it in today.
Status: NEW → ASSIGNED
Peter: Don't we need the check !mBody in processing style tag?
Yes, I think you're right Harish. Weird, I don't think we've ever had these
checks before.
With the patch ( id=37165 ) and I don't get the assertions.
Peter: 
+   if (!mInsideNoXXXTag && !mBody && NS_SUCCEEDED(rv) && src.IsEmpty()) {

The above would break backwards compatibility right? Because we want to load 
style whether the style tag is within the head or not.

I'm not sure if this helps, but anyway..

I cannot reproduce any of the "Save As" dialogs with build 2001060508 under
Linux. I do not have java or flash plugins installed and refused all the cookies
the sites tried to set.
Since there's an issue of reproducibility here, I wanted to note that I always
get the UCTH dialog when going to http://www.ashford.com. I'm using the 0.9.1
branch build on WinNT.
O.K., I can reproduce it on http://www.ashford.com with build 2001060508 and
build 2001060509.
After talking to Harish about this, I think my patch shouldn't go in. It helps
avoid this bug, but it would cause backwards compatibility problems. We've
always loaded style links (both through <style src=""> and <link href="">)
inside the body and my patch would break that. We should find out what check-ins
from before the 17th could have caused this. Why did the loading of those linked
stylesheets (and the full frame reconstruct?) not cause problems before?
I'm going to clear the 0.9.1 milestone. If anyone disagrees, I'm sorry but I
won't be around anymore to respond tonight.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
If I load ashford.com content from my hard disk it works!
Harish and I talked about this bug and he is gonna come back in from home to
work on this tonight.  Based on our discussion, we have a couple of options:

1) Check in Peter's version 2 patch
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37165) that disables
link elements outside of the head of a document.  This is the correct behavior
according to the HTML 4.0 spec but it will break backwards compatibility because
we have allowed link elements anywhere in a document in the past.  There might
be pages that display properly in Netscape 4.x/6.0 that will start displaying
with incorrect style once this patch is checked in.

2) Take more time and figure out the real fix to this bug.  Unfortunately, at
this point we don't have a clear idea of how long a time that is.

Harish plans to test the top 100 sites with Peter's version 2 patch and update
this bug with what he finds.  Subsequently, we'd like the PDT and
drivers@mozilla.org to decide whether this patch should get checked in or not.
Back to M0.9.1 pending Harish's report.  Nisheeth is lining up reviews in case
the fix checks out to be good.
Target Milestone: mozilla0.9.2 → mozilla0.9.1
r=harishd for patch v2 [ Though this would break backwards compatibility ]

I'll be running top 100 sites to evaluate the damage.
FYI: patch v2 does not seem to fix the problem seen in ashford.com!!!!
Hmm. So some debugging shows a number of problems here, none of which seem to be 
directly related to linked stylesheets.

1. www.ashford.com sends the page with a MIME type that uses mixed case:
	Content-Type: Text/HTML  
   this causes us to bail in a bunch of places where we do case sensitive
   string compares with "text/html".

   Maybe we should lowercase the MIME type in the channel up-front?

2. www.marketwatch.com does a bunch of redirects and meta-refreshes. I hit
   this assertion:

###!!! ASSERTION: NS_ENSURE_TRUE(mDocShell) failed: 'mDocShell', file 
nsDSURIContentListener.cpp, line 100

   before I started to see download dialogs, suggesting that interactions
   of refreshes with the creation/destruction of docShells cause us to try
   to load when the content listeners's docShell has been nulled out, with
   the result that we bail, and put up the download dialog.

CC'ing Netlib people.  Please take a look at this 0.91 bug, thanks!
the Content-Type lowercase bug is bug 83611
I tested the patch v2 against 110 urls and the following seems to get affected 
by the change:

http://www.go2net.com ( No visual difference ).
http://www.vserver.com ( No visual difference ).
http://www.mtv.com ( looks different - all styling lost! ).
So it seems that, long-term, we need to fix two things here:
1. Take the <link> in body fix, but perhaps conditionalize it to apply only
   when in quirks mode.

2. Ensure that when <iframe>s are being torn down (in this case because of
   frame reconstruction on style resolution), that we cancel any outstanding
   network requests, and avoid the null docShell problem.

Short-term, it seems we need the links fix all the time (quirks and strict 
modes).

sr=sfraser on v2 of the patch, if you're willing to suffer from backward 
compatibility bustage reported by harish.

how bad are the side effects on mtv.com, and what it the likleyhood
they would surface elsewhere?
Whiteboard: MUST HAVE; no eta yet → MUST HAVE; have patch with side effects on mtv.com
Font size/style/color, link style/color, and column background color is
incorrect in multiple places on the page.  But, the entire page is readable. 
About to attach screenshots that compare how the page looks in IE 5.0 versus a
Netscape 6  build with the patch.
Whiteboard: MUST HAVE; have patch with side effects on mtv.com → MUST HAVE; no eta yet
I forgot to mention when I attached the first screenshot
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37326) that it is a
Winzipped file.
I can't ready the first one. can you zip it too?
talked with nisheeth some more...  sounds like we really need
to let the the patch bake some more, do some more testing
and and sort out the right fix for the style related problems
that are making the save as dialog pop...

nitheeth is continuing to work on this.

it also sounds like we need
http://bugzilla.mozilla.org/show_bug.cgi?id=83611 for some
other sites like ashford.com if we want to fix all the conditions
where the dialog might pop.any chance 83611 can be moved in to 0.9.1,
 or will it take a while to get that fix ready?

asked about 83611 in that bug.
here's another site for your testing:

http://windowsupdate.microsoft.com/

Darin, I see that the docshell tells the doc loader to stop pending network 
requests when it is destroyed.  The doc loader aborts the load with:

  mLoadGroup->Cancel(NS_BINDING_ABORTED);

Later, the http channel fires the on start notification which bubbles up through 
nsDocumentOpenInfo into nsDSURIContentListener::DoContent().  But, mDocShell is 
null for the content listener so we end up aborting and throwing up a save as 
dialog.  

Is it correct for the on start notification to fire even after the doc loader 
has aborted pending loads?
Just spoke to Gagan and he threw out the possibility that the on start necko 
event might already be in the queue when the load group is cancelled.  He 
recollected hazily that Rick Potts had put in a workaround to ensure that 
pending necko events do not get processed after the load group is cancelled.

Rick, any help would be greatly appreciated.  I am gonna continue to try and 
verify this pending necko event hypothesis.
Adding rpotts to the cc list.  Rick, please read my earlier comment.  Thanks!
Chris, try saving the first screenshot with a .zip extension.  Then open the 
.zip file and you should see ns6.bmp in it.  I just tried it on my machine and 
it worked.
nisheeth: I don't know the details of the workaround gagan spoke of, but I can
tell you that OnStartRequest is always supposed to fire before OnStopRequest,
and that Cancel should force an immediate (but usually asynchronous)
OnStopRequest.  So, IMO it is normal to expect an OnStartRequest after a call
to Cancel if it is the case that OnStartRequest has not already been fired.
Not waiting for this right now -> 092
Target Milestone: mozilla0.9.1 → mozilla0.9.2
*** Bug 84358 has been marked as a duplicate of this bug. ***
Maybe in the OnStartRequest we should do IsPending() on the request and bail if
it has been cancelled?  I don't know if there's code that already tries to do
that, or, if there's cases where IsPending() is false for legitimate requests.
sounds like a good idea to me.
Okay, I have a one liner fix to nsHttpChannel that fixes the marketwatch problem 
the right way.  The status parameter was passed into nsHttpChannel::Cancel() but 
wasn't assigned into mStatus.  There is code upstream that does not propagate 
the OnStartRequest notification if the channel's status is set to cancelled.

Darin, would you review the patch I'm about to attach.  Vidur, would you 
super-review?  Thanks!
sr=darin (oh yeah!)
OK, since Darin has given an sr, Vidur gives this an r.  I just showed this to 
him and he was ok with it.

Pulling into 0.9.1 and will email drivers@mozilla.org for approval.
Target Milestone: mozilla0.9.2 → mozilla0.9.1
Taking...
Assignee: peterv → nisheeth
Status: ASSIGNED → NEW
Nisheeth, could you test if this fixed the crash in bug 84353 as well? (Sorry,
don't have a current build at hand...)
*** Bug 84353 has been marked as a duplicate of this bug. ***
Jussi, yes bug 84353 is fixed too.  Just marked it a dup.
Blocks: 83989
*** Bug 84096 has been marked as a duplicate of this bug. ***
a=chofmann
hoping nisheeth or some one is still around tonight and can get the
patch in by the next branch build spin at 3:00am..
the patch looks simple enough that anybody with check-in permission can check-in.
Thanks Nisheeth! :-)

Copying keywords from bug 84353: crash, mailtrack.
Keywords: crash, mailtrack
I'm more than happy to check this into the branch if no one minds. 
do it!  thanks
Whiteboard: MUST HAVE; no eta yet → MUST HAVE;
I checked this patch into both the branch and the tip.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Great detective work (and hard work at that!)  Thanks everyone!
*** Bug 84315 has been marked as a duplicate of this bug. ***
Verified on
build: 2001-06-20-04-Trunk
platform: Win NT

I do not see any save-as dialogs now, when I open above url.
Status: RESOLVED → VERIFIED
I'm gonna reopen this bug.  I saw this recently using 8-13-01 commercial trunk 
build on Win 2K. I saw once in cbsmarketwatch.com and today in cnn.com.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

    
    

    
  
I should also note that this happens all the times (atleast now) for cnn.com. 
Can someone confirm this? Thanks!

    
    

    
  
i've seen this using a debug linux build on cnn.com... it is always the same
exact URL from the timewarner hat.

    
    

    
  
wfm 2001081715 linux,
since 0.9.1 was a long time ago =)
-> future
Target Milestone: mozilla0.9.1 → Future

    
    

    
  
As this was reopened due to Save As on cnn.com and the recently fixed bug 96029
was exactly about that - compare the 08/20 23:27 bonsai comment "bug #96029
(r=valeski, sr=mscott) Loading cnn.com caused the sav-as dialog to appear..." -
I believe we mark this bug again as fixed. Looking for verified.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED

    
    

    
  
Marking verified as per above developer comments. When I open attached URL, the
Save As.. dialog does not appear anyhmore.
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: