[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 bugzilla-request-daemon at opendarwin.org
Sun Apr 2 16:28:43 PDT 2006

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

Attachment 7459: revised fix

------- 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.

More information about the webkit-reviews mailing list