231383 - txStylesheetCompiler.cpp should not error on NS_FAILED(mRV)

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Description

[Axel Hecht]

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.

Comment 1

[Axel Hecht]

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.

Comment 2

[Axel Hecht]

Attachment: 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.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 4

[Axel Hecht]

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 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 6

[Peter Van der Beken [:peterv]]

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.

Comment 7

[Axel Hecht]

fix checked in, feb, 06, 3:25