Closed
Bug 1148711
Opened 9 years ago
Closed 9 years ago
nsCSSParser::Parse{Property,Variable} always return NS_OK
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: heycam, Assigned: heycam)
Details
Attachments
(1 file)
11.85 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
Let's remove the nsresult return values from nsCSSParser::Parse{Property,Variable}, as they always return NS_OK.
Attachment #8584969 -
Flags: review?(dholbert)
Comment 1•9 years ago
|
||
Comment on attachment 8584969 [details] [diff] [review] patch Review of attachment 8584969 [details] [diff] [review]: ----------------------------------------------------------------- r=me, just one comment nit: ::: layout/style/nsDOMCSSDeclaration.cpp @@ +337,5 @@ > if (decl != olddecl) { > delete decl; > } > + // Don't throw an exception if there was a parse error. > + return NS_OK; The wording here is a bit ambiguous & might benefit from a tweak. Right now, it can be mis-interpreted as: "Now we're maybe going to throw an exception, except don't do that if there was a parse error". But really what you're trying to say is, *we know there was a parse error*, and we don't throw for that. Maybe reword something like this, if this matches the meaning you're going for: // Parsing failed -- but we don't throw an exception for that. (You have this code-comment in two different places -- if you tweak it, make sure to change both of them.
Attachment #8584969 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 2•9 years ago
|
||
Comment tweak sounds fine, thanks. https://hg.mozilla.org/integration/mozilla-inbound/rev/5fe056ac3907
Comment 3•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5fe056ac3907
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in
before you can comment on or make changes to this bug.
Description
•