Closed
Bug 1148711
Opened 10 years ago
Closed 10 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•10 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•10 years ago
|
||
Comment tweak sounds fine, thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fe056ac3907
Comment 3•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 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
•