[webkit-reviews] review denied: [Bug 8351] display:none has no effect on <option> element : [Attachment 8980] First attempt

[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 8351: display:none has no effect on <option> element
http://bugzilla.opendarwin.org/show_bug.cgi?id=8351

Attachment 8980: First attempt
http://bugzilla.opendarwin.org/attachment.cgi?id=8980&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Ideally this would not be done inside the renderer since the renderer is going
away, but I suppose Adele can just consider it when re-implementing it.

It seems very strange to check the style attached to an object directly without
doing any style resolution. If we are going to do it, we should not be
converting it to a string. We should be getting the CSSValue object and then
calling isPrimitiveValue(), casting to a CSSPrimitiveValue, calling getIdent()
on it, and then checking against CSS_VAL_NONE.

Besides not doing any style resolution, it also seems strange to single out
display:none and not implement any other properties, like visibility:hidden or
other stranger display values.

I'm going to review- this because at the very least we should use
getPropertyCSSValue instead of getPropertyValue.