[webkit-reviews] review denied: [Bug 13097] Unsupported content:inherit value : [Attachment 53180] Rounds off support for CSS2.1 content

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Nicholas Wilson
<nicholas at nicholaswilson.me.uk>'s request for review:
Bug 13097: Unsupported content:inherit value
https://bugs.webkit.org/show_bug.cgi?id=13097

Attachment 53180: Rounds off support for CSS2.1 content
https://bugs.webkit.org/attachment.cgi?id=53180&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good.

> +	   if (id == CSSValueNormal ||
> +	       id == CSSValueNone)
> +	       validPrimitive = true;
> +	   else {
> +	       return parseContent(propId, important);
> +	   }

No braces around a single-line else. No need to break that up into multiple
lines. I suggest this formatting:

    if (id == CSSValueNormal || id == CSSValueNone)
	validPrimitive = true;
    else
	return parseContent(propId, important)
    break;

review- because this patch includes a new regression test but does not include
the expected result for the test

It would also be best if the test could be changed so that it can be dumpAsText
instead, as we have done for many style tests in the past. But what's mandatory
is including the expected results.