Closed
Bug 88423
Opened 23 years ago
Closed 21 years ago
Remove NS_CSS_PARSER_DROP_DECLARATION
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: bzbarsky, Assigned: dbaron)
References
Details
Attachments
(2 files)
1.92 KB,
patch
|
Details | Diff | Splinter Review | |
3.69 KB,
patch
|
Details | Diff | Splinter Review |
BUILD: 2001-06-28 source If one calls CSSParserImpl::ParseDeclaration with the declaration "font-style: bold", then the return value is PR_FALSE and aErrorCode is NS_OK. It would make more sense to have the error code be NS_CSS_PARSER_DROP_DECLARATION. The problem seems to be that CSSParserImpl::ParseVariant will return PR_FALSE and leave aErrorCode as NS_OK in this case. Perhaps CSSParserImpl::ParseDeclaration should make sure that aErrorCode is set to NS_CSS_PARSER_DROP_DECLARATION any time it drops a declaration? Something like |if (aErrorCode == NS_OK) aErrorCode = NS_CSS_PARSER_DROP_DECLARATION| in all the cases when it returns PR_FALSE?
Reporter | ||
Comment 1•23 years ago
|
||
To clarify. This is the function with the prototype: CSSParserImpl::ParseDeclaration(PRInt32& aErrorCode, nsICSSDeclaration* aDeclaration, PRBool aCheckForBraces, PRInt32& aChangeHint)
Reporter | ||
Comment 2•23 years ago
|
||
Reporter | ||
Updated•23 years ago
|
Assignee | ||
Comment 3•23 years ago
|
||
IIRC, we should *never* return NS_CSS_PARSER_DROP_DECLARATION since it means DROP_DECLARATION_BLOCK.
Assignee | ||
Comment 4•23 years ago
|
||
Why is a parsing error an exceptional circumstance anyway? We're responding to user input, and should handle it gracefully, as I think we do.
Assignee | ||
Comment 5•23 years ago
|
||
FWIW (IIRC), the NS_CSS_PARSER_DROP_DECLARATION error code was created due to a misunderstanding but never removed from the codebase as it probably should have been.
Comment 6•23 years ago
|
||
David is correct. Renaming the bug to "Remove NS_CSS_PARSER_DROP_DECLARATION" from "CSSParserImpl::ParseDeclaration can return false and leave aErrorCode as NS_OK"...
Comment 7•23 years ago
|
||
Comment 8•23 years ago
|
||
david, marc: please r/sr...
Comment 9•23 years ago
|
||
I'm not sure I understand why you can just drop the code in GenericElement and nsCSSStyleRule that calls SetCSSDeclaration(declClone); Is it because the return code NS_CSS_PARSER_DROP_DECLARATION is never set in the first place, so that code can never be executed? If that is the case, [s]r=attinasi , otherwise please clarify my murky understanding.
Reporter | ||
Comment 10•23 years ago
|
||
The code that sets the declaration to declClone is there for the following reason, as far as I can tell: The DOM CSS spec requires that setting .cssText throw a DOM_SYNTAX_ERR exception when the string it's being set to is "unparsable". What this means is not clear, but throwing an exception means you don't change the actual cssText or the declaration, so you have to throw away your changes (including any clearing that happened). The working group has been contacted for a clarification of this by dbaron, but I don't think they've said anything useful yet.
Comment 11•23 years ago
|
||
Reassigned to dbaron. Depending on what the WG decides, we may have to either apply the patch above (+ remove the declaration of 'declClone' that I had forgotten), or restore the original declaration if ParseAndAppendDeclaration() returns an error: if (aClearOldDecl && NS_FAILED(result)) { SetCSSDeclaration(declClone); }
Assignee: pierre → dbaron
Status: ASSIGNED → NEW
Summary: Remove NS_CSS_PARSER_DROP_DECLARATION → [WG]Remove NS_CSS_PARSER_DROP_DECLARATION
Target Milestone: --- → Future
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: Future → mozilla0.9.4
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Assignee | ||
Updated•23 years ago
|
Priority: -- → P4
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Assignee | ||
Updated•23 years ago
|
Priority: P4 → P3
Target Milestone: mozilla0.9.7 → Future
Assignee | ||
Updated•21 years ago
|
Depends on: 197212
Summary: [WG]Remove NS_CSS_PARSER_DROP_DECLARATION → Remove NS_CSS_PARSER_DROP_DECLARATION
Reporter | ||
Comment 12•21 years ago
|
||
Fixed by bug 197212 checkin -- NS_CSS_PARSER_DROP_DECLARATION is no more.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•