txDriver::startElement and friends should check mRV first instead of calling into cancel over and over when there is a NS_FAILED(mStatus) in mCompiler.
adjusting topic, I reconsidered, as this touched both standalone and module. txStylesheetCompiler should just return NS_OK if mRV is an error code. That way, we only cancel once and the fact that the stylesheet compiler ignores content fed into it after an error is not really an error in the first place. Patch coming up. I'll fix up some logging stuff on the way, this is how I found this problem.
Status: NEW → ASSIGNED
Hardware: PC → All
Summary: txDriver in txStandaloneStylesheetCompiler.cpp doesn't check mRV → txStylesheetCompiler.cpp should not error on NS_FAILED(mRV)
Target Milestone: --- → mozilla1.7alpha
Created attachment 139414 [details] [diff] [review] return NS_OK on NS_FAILED(mRV), better prlog info I moved the PR_LOG for the stacks inside of txStylesheetCompilerState into #ifdef TX_DEBUG_STACK so that one can switch on that debugging info if one really needs it. That stuff is pretty baked by now and adds alot of noise in general.
I don't get it, why wouldn't you want to return an error if the compiler is canceled or has failed? If other classes end up calling cancel on the compiler multiple times because they keep trying to feed a failed compiler with more events then that sounds like a bug in those classes? r=me on the logging changes though.
The problem is that expat doesn't handle errors in element handlers, and keeps on feeding content into the sink 'til it's buffer is empty. So we need to wallpaper against that, and, as we need it for both module and standalone and as the txStylesheetCompiler handles mRV already, I decided that that place was the right one.
Comment on attachment 139414 [details] [diff] [review] return NS_OK on NS_FAILED(mRV), better prlog info Ok, it'd be nice to revert this once/if we move to an expat that does do errorhandling though.
Attachment #139414 - Flags: review?(bugmail) → review+
Comment on attachment 139414 [details] [diff] [review] return NS_OK on NS_FAILED(mRV), better prlog info I'd prefer comments that make it clear that we want to revert this if expat allows us to stop.
Attachment #139414 - Flags: superreview?(peterv) → superreview+
fix checked in, feb, 06, 3:25
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.