[webkit-reviews] review denied: [Bug 7118] Property values with extra items do not get treated as invalid (they should) : [Attachment 7459] revised fix

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 7118: Property values with extra items do not get treated as invalid (they
should)
http://bugzilla.opendarwin.org/show_bug.cgi?id=7118

Attachment 7459: revised fix
http://bugzilla.opendarwin.org/attachment.cgi?id=7459&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I think this patch is great; headed in the right direction, but not ready to
land yet.

Why is there any need to check "num" in functions that call parseShorthand? It
seems to me that parseShorthand is already guaranteed to do the right thing; it
can't possibly return true if there are more values than properties.

Could we perhaps fix the whole bug another way? Instead of looking at
valueList->size(), could we look at valueList->current() at the end of the
function to see if it's 0? That seems to go more with the flow of the code.
Seems like that would work for everything except for the very few places that
explicitly call return true.

What about the CSS_VAL_INHERIT and CSS_VAL_INITIAL cases? Don't those need one
of these checks?

What about the SVG properties? Those should have the same issue, but they're in
a different source file.