Status
()
People
(Reporter: axel, Assigned: axel)
Tracking
({crash, regression})
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(1 attachment, 1 obsolete attachment)
2.23 KB,
patch
|
peterv
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
the nsresult returned by createExpr should be either a error value or a non-null expr, no mixture. This means that fcp needs to be handled outside of createExpr in txStylesheetCompileHandlers.cpp, getExprAttr.
Any progress on this? This is a pretty serious regression since crashers were introduced.
Keywords: crash, regression
err.. pressed submit prematurly. 1.6a is comming up in a couple of days and it'd suck to have that released with known xslt regressions.
(Assignee) | ||
Comment 3•16 years ago
|
||
Created attachment 133755 [details] [diff] [review] fixing the leak
(Assignee) | ||
Comment 4•16 years ago
|
||
I don't see a crash anywhere. We never return NS_OK for a null expression. This code leaked, though. Note, I'm not doing fcp. I don't expect to get reviews on anything I do, so I'm not doing anything. Rationale for my opinion: fcp is pretty useless in its own right, xslt1.1 is dead and xslt2 is just too far away to be actually of any use in a 1.0 processor. I'd only do a single expression, hooked up in getExprAttr to get fcp done, which conflicts with the language of the fcp spec as far as function argument counts go. That says "should only error if the function is called". Of course the only way for that not to happen in a expr evaluation would be a (function-available(foo) and foo('way', 'too', 'many', 'arguments')). Of course that's pretty useless in most cases, as that works only for boolean function call results *and* under the assumption that false is a good default value. It's just silly language stating something completely irrelevant. Not gonna go on my time.
Status: NEW → ASSIGNED
Keywords: crash
(Assignee) | ||
Updated•16 years ago
|
Attachment #133755 -
Flags: superreview?(peterv)
Attachment #133755 -
Flags: review?(bugmail)
Updated•16 years ago
|
Attachment #133755 -
Flags: superreview?(peterv) → superreview+
it returns NS_OK and a null-expression for the empty string so the current patch only fixes part of this bug.
Keywords: crash
Attachment #133755 -
Flags: review?(bugmail) → review+
Comment on attachment 133755 [details] [diff] [review] fixing the leak upps, that should be |*aExpr| and not |aExpr|
Attachment #133755 -
Flags: review+ → review-
Created attachment 133780 [details] [diff] [review] patch to fix The change to the lexer makes the lexer treat "" as " ", which then the parser catches as a parse-error
Attachment #133755 -
Attachment is obsolete: true
Updated•16 years ago
|
Attachment #133780 -
Flags: superreview+
Attachment #133780 -
Flags: review+
checked in, thanks for reviews
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•