Closed
Bug 241300
Opened 20 years ago
Closed 20 years ago
Crash with with firefox on this url with invalid background [@ nsImageLoader::Load ]
Categories
(Core :: Layout, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.8beta1
People
(Reporter: martijn.martijn, Assigned: bzbarsky)
References
()
Details
(4 keywords)
Crash Data
Attachments
(2 files, 1 obsolete file)
103 bytes,
text/html
|
Details | |
2.41 KB,
patch
|
Biesinger
:
review+
darin.moz
:
superreview+
asa
:
approval-aviary+
dveditz
:
approval1.7.5+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040420 Firefox/0.8.0+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040420 Firefox/0.8.0+ From the Mozillazine forums: http://forums.mozillazine.org/viewtopic.php?t=70726&sid=4913fcda6a319ed8b94887ccfa1fed85 This url is reliably crashing Firefox (not Mozilla). The minimal testcase is this: <html><head></head> <body background="cid:00d201c264d0$feb75c80$0300a8c0@node3"> </body> </html> Not crashing in: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040305 Firefox/0.8.0+ Crashing in: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040309 Firefox/0.8.0+ Reproducible: Always Steps to Reproduce: 1.Visit the url 2. 3. Actual Results: Crash Expected Results: No crash
Reporter | ||
Comment 1•20 years ago
|
||
Reporter | ||
Comment 2•20 years ago
|
||
Sorry, I forgot to mention that for the testcase to crash, you have to resize the window.
Comment 3•20 years ago
|
||
Martijn: Could you provide TalkBack incident ID?
Severity: normal → critical
Keywords: crash
Updated•20 years ago
|
Keywords: regression
Reporter | ||
Comment 4•20 years ago
|
||
(In reply to comment #3) > Martijn: Could you provide TalkBack incident ID? Sorry, no, afaik there are yet no firefox builds on windows with Talkback enabled. However, I heard there are Talkback enabled builds on Linux.
Comment 5•20 years ago
|
||
I've got a linux build with talkback, but I can't reproduce the bug... WorksForMe with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7b) Gecko/20040415 Firefox/0.8.0+
Comment 6•20 years ago
|
||
i opened the bug in a new tab (without resizing anything) and it DID crash. I use 20040331 nighty.
Comment 7•20 years ago
|
||
Confirming and marking new : Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a) Gecko/20040416 Firefox/0.8.0+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 8•20 years ago
|
||
I've been able to compile a firefox debug build. Just before the crash I get an assertion (with the testcase and the url): You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr!=0', file ../../../../dist/include/xpcom/nsCOMPtr.h line 711
Comment 9•20 years ago
|
||
Confirmed crash on testcase when resizing window. - Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040503 Firefox/0.8.0+ - Microsoft Windows 2000 Pro 5.00.2195 SP4
Comment 10•20 years ago
|
||
The url given WFM Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a) Gecko/20040517 Firefox/0.8.0+
Reporter | ||
Comment 11•20 years ago
|
||
Well, my 2004/05/12 Firefox build seems to have Talkback enabled. This is the Talkback ID: TB52084G
Reporter | ||
Comment 12•20 years ago
|
||
With my debug Firefox build from 2004/05/17 I don't get an assertion anymore, but still a crash. The GDB console shows me this: Program received signal SIGSEGV, Segmentation fault. nsImageLoader::Load(imgIRequest*) (this=0x83024b0, aImage=0x7fcf320) at ../../../dist/include/xpcom/nsCOMPtr.h:710 The second time, this: nsImageLoader::Load(imgIRequest*) (this=0x7eda8e8, aImage=0x7d19ff0) at ../../../dist/include/xpcom/nsCOMPtr.h:710
Comment 13•20 years ago
|
||
wfm FF 20040604 trunk on Linux and Seamonkey debug build 20040604. Anyone can check if still crashing and attach a full stacktrace or Talkback ID ?
Summary: Crash with with firefox on this url with invalid background → Crash with with firefox on this url with invalid background [@ nsImageLoader::Load ]
Reporter | ||
Comment 14•20 years ago
|
||
It still crashes with me: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/2004-06-06 Firefox/0.8.0+ No Talkback agent is bundled with the nightlies, so I can't give a Talkback ID. With the debug build, I still get the same gdb messages. The testcase doesn't crash that easily. The most likely circumstance in which it crashes: -Save it locally. -Open it and press the 'Go'-button a few times.
Comment 15•20 years ago
|
||
It crashes for me using Firefox 0.9.1 (inst. build, WinXP SP1/current, Talkback-enabled, no extension, no theme) Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1 I cannot give any TB ID however. Is it because I'm behind an HTTP proxy ? Here's what I did - save the testcase locally. Quit Fx. - launch Fx, type the URL file:///d:/tmp/bug_241300.htm and [RETURN]. - Click the URL to select it anew. [RETURN] a 2nd time. Crash. - Talkback appears, Send, TB exits (no TB error message, no ID).
Comment 16•20 years ago
|
||
Martijn: To get the Talkback ID's go to your Firefox directory and go to components/, there start talkback.exe, it'll give you a list with the IDs.
Comment 17•20 years ago
|
||
Thanks Adam, indeed I have the ID now. (I assume that your comment was addressed to me.) I did just the same, this time with 0.9.1-release. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1 There's an interesting thing (I think). I recall the sequence : 1. Launch Fx, type the local URL for the testcase, [RETURN]. Blank page. 2. Click to select the URL just typed, [RETURN]. Nothing happens. Again a blank page. 3. Click on it (the blank page), just to see. Nothing happens. But ... 4. Click on the URL bar, as if I wanted to repeat step 2, and there's the crash ! The crash does not happen right after step 2. Talkback ID TB193939W 2004-06-29 23:24 GMT+02 , sent.
Comment 18•20 years ago
|
||
David, Boris: This looks like regression from bug 57607 changes (regression range is between 20040305 and 20040309, check-in is 2004-03-08). What do you think?
Comment 19•20 years ago
|
||
*** Bug 248315 has been marked as a duplicate of this bug. ***
Comment 20•20 years ago
|
||
This bug also affects Thunderbird - see bug 248315. -> Layout
QA Contact: firefox.general
Updated•20 years ago
|
Assignee: firefox → nobody
Component: General → Layout
Product: Firefox → Browser
QA Contact: firefox.general → core.layout
Version: unspecified → Trunk
Reporter | ||
Comment 21•20 years ago
|
||
I don't know if this is right, but this makes the testcase not crash anymore.
Comment 22•20 years ago
|
||
nsCOMPtr<nsIURI> oldURI; + if (!oldURI) + return NS_ERROR_FAILURE; that is wrong. there is _never_ a case where oldURI is non-null at this point.
Comment on attachment 146752 [details]
This will crash!
This testcase doesn't crash for me in Linux Firefox on either the Trunk or the
Aviary 1.0 branch.
Assignee | ||
Comment 24•20 years ago
|
||
Do you have to have "cid:whatever" to crash? Does using "foo:bar" crash also? If not, does it matter what comes after the "cid:" part?
Reporter | ||
Comment 25•20 years ago
|
||
(In reply to comment #24) > Do you have to have "cid:whatever" to crash? Does using "foo:bar" crash also? > If not, does it matter what comes after the "cid:" part? "cid:foo" does indeed crash (the new url's testcase crashes), but "foo:bar" does not crash, so I don't think it does matter much what comes after the "cid:" part.
Reporter | ||
Comment 26•20 years ago
|
||
This is the output from the command 'bt' of gdb: #0 0x77f813b2 in ?? () #1 0x67834260 in nsDebugImpl::Break(char const*, int) (this=0x198be80, aFile=0x36fefa0 "../../../dist/include/xpcom/nsCOMPtr.h", aLine=711) at c:/mozilla/mozilla/xpcom/base/nsDebugImpl.cpp:306 #2 0x6783416f in nsDebugImpl::Assertion(char const*, char const*, char const*, int) (this=0x198be80, aStr=0x36fefe0 "You can't dereference a NULL nsCOMPtr with operator->().", a Expr=0x36fefc7 "mRawPtr != 0", aFile=0x36fefa0 "../../../dist/include/xpcom/nsCOMPtr.h", aLine=711) at c:/mozilla/mozilla/xpcom/base/nsDebugImpl.cpp:286 #3 0x6785d8d6 in nsDebug::Assertion(char const*, char const*, char const*, int) (aStr=0x36fefe0 "You can't dereference a NULL nsCOMPtr with operator->().", aExpr=0x36fefc7 "mRawPtr != 0", aFile=0x36fefa0 "../../../dist/include/xpcom/nsCOMPtr.h", aLine=711) at c:/mozilla/mozilla/xpcom/glue/nsDebug.cpp:108 #4 0x036ff3fa in nsImageLoader::Load(imgIRequest*) (this=0xf513918, aImage=0xf45e548) at ../../../dist/include/xpcom/nsCOMPtr.h:711 #5 0x03705130 in nsPresContext::LoadImage(imgIRequest*, nsIFrame*, imgIRequest* *) (this=0xf4ea6e0, aImage=0xf45e548, aTargetFrame=0xed68ea4, aRequest=0x22e640) at c:/mozilla/mozilla/layout/base/src/nsPresContext.cpp:985 #6 0x0366d6d0 in nsCSSRendering::PaintBackgroundWithSC(nsIPresContext*, nsIRend eringContext&, nsIFrame*, nsRect const&, nsRect const&, nsStyleBackground const& , nsStyleBorder const&, nsStylePadding const&, int, nsRect*) ( aPresContext=0xf4ea6e0, aRenderingContext=@0xed41388, aForFrame=0xed68ea4, aDirtyRect=@0x22e9a0, aBorderArea=@0x22e780, aColor=@0x22e6f0, aBorder=@0xee42618, aPadding=@0xee42b40, aUsePrintSettings=1, aBGClipRect=0x0) at ../../../../dist/include/xpcom/nsCOMPtr.h:704 #7 0x0366cf05 in nsCSSRendering::PaintBackground(nsIPresContext*, nsIRenderingC ontext&, nsIFrame*, nsRect const&, nsRect const&, nsStyleBorder const&, nsStyleP adding const&, int, nsRect*) (aPresContext=0xf4ea6e0, aRenderingContext=@0xed41388, aForFrame=0xed68ea4, aDirtyRect=@0x22e9a0, aBorderArea=@0x22e780, aBorder=@0xee42618, aPadding=@0xee42b40, aUsePrintSettings=1, aBGClipRect=0x0) at c:/mozilla/mozilla/layout/html/style/src/nsCSSRendering.cpp:2787 I can give more of this output, if needed.
Assignee | ||
Comment 27•20 years ago
|
||
What are the values of the locals and member variables in frame 4?
Assignee | ||
Comment 28•20 years ago
|
||
Martijn, thanks for the debug data! Could someone seeing the crash try this patch?
Assignee | ||
Updated•20 years ago
|
Attachment #152193 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #153222 -
Flags: superreview?(darin)
Attachment #153222 -
Flags: review?(cbiesinger)
Comment 29•20 years ago
|
||
Comment on attachment 153222 [details] [diff] [review] Proposed patch NS_NEWXPCOM(channel, nsExtProtocolChannel); if (!channel) return NS_ERROR_OUT_OF_MEMORY; ((nsExtProtocolChannel*) channel.get())->SetURI(aURI); + ((nsExtProtocolChannel*) channel.get())->SetOriginalURI(aURI); ideally this'd be constructor arguments... But in any case, setting originalURI does not need a cast.
Attachment #153222 -
Flags: review?(cbiesinger) → review+
Reporter | ||
Comment 30•20 years ago
|
||
(In reply to comment #28) > Could someone seeing the crash try this patch? I've applied the patch and did a rebuild and now I don't see a crash anymore with the testcase.
Comment 31•20 years ago
|
||
Comment on attachment 153222 [details] [diff] [review] Proposed patch >Index: uriloader/exthandler/nsExternalProtocolHandler.cpp > NS_IMETHODIMP nsExtProtocolChannel::GetOriginalURI(nsIURI* *aURI) > { >+ NS_PRECONDITION(aURI, "Must have original URI!"); >+ NS_IF_ADDREF(*aURI = mOriginalURI); > return NS_OK; > } i'd probably remove the NS_PRECONDITION since it doesn't seem to add much. we'd crash if we hit that assertion, and it would be pretty obvious that the crash occured because the input arg was null. also the text of the assertion doesn't seem to make sense to me.
Attachment #153222 -
Flags: superreview?(darin) → superreview+
Assignee | ||
Updated•20 years ago
|
Assignee: nobody → bzbarsky
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta
Assignee | ||
Comment 32•20 years ago
|
||
Checked in, with review comments addressed.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 33•20 years ago
|
||
*** Bug 254907 has been marked as a duplicate of this bug. ***
Comment 34•20 years ago
|
||
This didn't make the Aviary branch apparently. See bug 250917 (which can probably be duped here)
Flags: blocking-aviary1.0?
Assignee | ||
Comment 35•20 years ago
|
||
*** Bug 250917 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 36•20 years ago
|
||
Comment on attachment 153222 [details] [diff] [review] Proposed patch This ought to be safe enough for the branches...
Attachment #153222 -
Flags: approval1.7.x?
Attachment #153222 -
Flags: approval-aviary?
Comment 37•20 years ago
|
||
Comment on attachment 153222 [details] [diff] [review] Proposed patch a=dveditz for 1.7.x
Attachment #153222 -
Flags: approval1.7.x? → approval1.7.x+
Comment 38•20 years ago
|
||
Comment on attachment 153222 [details] [diff] [review] Proposed patch a=asa for aviary checkin.
Attachment #153222 -
Flags: approval-aviary? → approval-aviary+
Updated•20 years ago
|
Flags: blocking-aviary1.0?
Updated•16 years ago
|
Flags: in-testsuite+
Updated•13 years ago
|
Crash Signature: [@ nsImageLoader::Load ]
You need to log in
before you can comment on or make changes to this bug.
Description
•