[webkit-reviews] review denied: [Bug 23477] Support for WCSS extensions : [Attachment 26929] initial patch for WCSS marquee styles

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied maheshK <mahesh.kulkarni at nokia.com>'s
request for review:
Bug 23477: Support for WCSS extensions
https://bugs.webkit.org/show_bug.cgi?id=23477

Attachment 26929: initial patch for WCSS marquee styles 
https://bugs.webkit.org/attachment.cgi?id=26929&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
I think Hyatt should review this.

> +    RenderStyle* s = m_layer->renderer()->style();
> +    
> +    if(s->marqueeLoopCount() == 0 && s->display() == WAP_MARQUEE)

WebKit coding style would use a longer name, perhaps "style" for the local
variable.

WebKit coding style would include a space after the word "if" before the "(".

WebKit coding style would normally use "!" rather than "== 0".

This patch adds lots of "wap" CSS properties, but it adds them unconditionally.
That means they can be tested with regression tests. So this needs to include
some regression tests demonstrating these properties are present.

This patch adds lots of "wap" CSS properties that it doesn't implement. Is that
really the right way to do this?

This patch adds some properties, like -wap-marquee-speed, without adding
computed style support. It's unfortunate that there are so many old properties
where we don't do computed style correctly. We don't want to make things even
worse by adding new properties that also don't have computed style support. So
that should be included with any new CSS properties.

I'm going to say review-, but Hyatt needs to review for the more basic
questions like, "Should we have these '-wap' properties unconditionally in all
WebKit-based browsers?"