txStylesheetCompiler.cpp should not error on NS_FAILED(mRV)

RESOLVED FIXED in mozilla1.7alpha



15 years ago
15 years ago


(Reporter: axel, Assigned: axel)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



15 years ago
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

15 years ago
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.
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

Comment 2

15 years ago
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 
so that one can switch on that debugging info if one really needs it. That
is pretty baked by now and adds alot of noise in general.


15 years ago
Attachment #139414 - Flags: superreview?(peterv)
Attachment #139414 - Flags: review?(bugmail)
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

15 years ago
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+

Comment 7

15 years ago
fix checked in, feb, 06, 3:25
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.