Closed
Bug 362716
Opened 18 years ago
Closed 18 years ago
[FIX]crash in [@ nsXBLPrototypeHandler::AppendHandlerText]
Categories
(Core :: XBL, defect, P1)
Core
XBL
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha1
People
(Reporter: arno, Assigned: bzbarsky)
References
()
Details
(Keywords: crash, verified1.8.0.10, verified1.8.1.2)
Crash Data
Attachments
(5 files)
477 bytes,
patch
|
Details | Diff | Splinter Review | |
299 bytes,
text/xml
|
Details | |
298 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
3.34 KB,
patch
|
sicking
:
review+
sicking
:
superreview+
|
Details | Diff | Splinter Review |
1.87 KB,
patch
|
jay
:
approval1.8.1.2+
jay
:
approval1.8.0.10+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.7) Gecko/20060830 Firefox/1.5.0.7 (Debian-1.5.dfsg+1.5.0.7-2) Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.7) Gecko/20060830 Firefox/1.5.0.7 (Debian-1.5.dfsg+1.5.0.7-2) Hi, I noticed a crash. It happens when in a xbl file you have a handler element with a command attribute if you are not in chrome. See url test case for an example. I investigated and discovered that that crash happens in nsXBLPrototypeHandler::AppendHandlerText (file content/xbl/src/nsXBLPrototypeHandler.cpp) in nsXBLContentSink::OnOpenContainer (file content/xbl/src/nsXBLContentSink.cpp) there is : if (command && !mIsChromeOrResource) // Make sure the XBL doc is chrome or resource if we have a command // shorthand syntax. return; // Don't even make this handler. so in case command attribute is set, and url scheme is not chrome (or ressource), mHandler is not created. but in nsXBLContentSink::FlushText (called from nsXBLContentSink::HandleEndElement), there is : if (mSecondaryState == eXBL_InHandler) { mHandler->AppendHandlerText(text); AppendHandler method of mHandler is called but mHandler has not been created (and is therefor still nsnuss). So, my browser crashes. I make a small patch that seems to solve the problem, but may we could issue a console warning or do something more appropriate. Reproducible: Always Steps to Reproduce: 1. go to http://ffsearchplugins.free.fr/bugzilla/example.xul Actual Results: crash Expected Results: should not crash :)
Reporter | ||
Comment 1•18 years ago
|
||
Reporter | ||
Updated•18 years ago
|
Component: General → XBL
Product: Firefox → Core
Version: unspecified → 1.8 Branch
Keywords: crash
Summary: crash in nsXBLPrototypeHandler::AppendHandlerText → crash in [@ nsXBLPrototypeHandler::AppendHandlerText]
Comment 2•18 years ago
|
||
As a wild guess I think it might be better if mSecondaryState didn't lie.
Assignee | ||
Comment 3•18 years ago
|
||
Yeah, definitely. The problem is that the current code (with or without the proposed null-check) will stick the text in the wrong handler if there's a handler preceding the "command" handler in question...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.8.0.x?
Flags: blocking1.9?
Flags: blocking1.8.1.2?
Hardware: PC → All
Version: 1.8 Branch → Trunk
Assignee | ||
Comment 4•18 years ago
|
||
Assignee | ||
Comment 5•18 years ago
|
||
Assignee | ||
Comment 6•18 years ago
|
||
Treat this situation as an error and bail on the binding. Also treat OOM constructing a handler as an error.
Attachment #247454 -
Flags: superreview?(bugmail)
Attachment #247454 -
Flags: review?(bugmail)
Updated•18 years ago
|
QA Contact: general → ian
Assignee | ||
Comment 7•18 years ago
|
||
I should note that there's no need to cc me on XBL bugs if you properly reassign them -- I watch the default XBL assignee. But you do need to properly reassign...
Assignee: nobody → bzbarsky
Priority: -- → P1
Summary: crash in [@ nsXBLPrototypeHandler::AppendHandlerText] → [FIX]crash in [@ nsXBLPrototypeHandler::AppendHandlerText]
Target Milestone: --- → mozilla1.9alpha
Attachment #247454 -
Flags: superreview?(bugmail)
Attachment #247454 -
Flags: superreview+
Attachment #247454 -
Flags: review?(bugmail)
Attachment #247454 -
Flags: review+
Assignee | ||
Comment 8•18 years ago
|
||
Fix checked in. Not sure what to do with branches; localization is frozen, right? I suppose I could just not report the error there (and silently bail).
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Flags: wanted1.8.0.x? → blocking1.8.0.10?
Comment 9•18 years ago
|
||
bz: Let's fix the crash and not worry about reporting the error for the branches.
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10?
Flags: blocking1.8.0.10+
Comment 10•18 years ago
|
||
Still need a branch patch
Assignee | ||
Comment 11•18 years ago
|
||
Attachment #250795 -
Flags: approval1.8.1.2?
Attachment #250795 -
Flags: approval1.8.0.10?
Comment 12•18 years ago
|
||
Comment on attachment 250795 [details] [diff] [review] Branch patch Approved for the branches, a=jay for drivers.
Attachment #250795 -
Flags: approval1.8.1.2?
Attachment #250795 -
Flags: approval1.8.1.2+
Attachment #250795 -
Flags: approval1.8.0.10?
Attachment #250795 -
Flags: approval1.8.0.10+
Updated•18 years ago
|
Keywords: fixed1.8.1.1 → fixed1.8.1.2
Comment 14•18 years ago
|
||
verified fixed on the 1.8.0 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10pre) Gecko/20070111 Firefox/1.5.0.10pre. No crash using http://ffsearchplugins.free.fr/bugzilla/example.xul. Also verified fixed on the 1.8 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.2pre) Gecko/20070109 BonEcho/2.0.0.2pre. No crash using http://ffsearchplugins.free.fr/bugzilla/example.xul. Adding the relevant verified keywords.
Comment 15•17 years ago
|
||
Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070207 Minefield/3.0a3pre
Status: RESOLVED → VERIFIED
Flags: blocking1.9?
Updated•13 years ago
|
Crash Signature: [@ nsXBLPrototypeHandler::AppendHandlerText]
You need to log in
before you can comment on or make changes to this bug.
Description
•