[webkit-reviews] review denied: [Bug 29508] Add ENABLE(ORIENTATION_EVENTS) : [Attachment 39796] patch to implement bug fix

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Greg Bolsinga <bolsinga at apple.com>'s
request for review:
Bug 29508: Add ENABLE(ORIENTATION_EVENTS)
https://bugs.webkit.org/show_bug.cgi?id=29508

Attachment 39796: patch to implement bug fix
https://bugs.webkit.org/attachment.cgi?id=39796&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
> +#if ENABLE(ORIENTATION_EVENTS)
> +EventListener* HTMLBodyElement::onorientationchange() const
> +{
> +    return
document()->getWindowAttributeEventListener(eventNames().orientationchangeEvent
);
> +}
> +
> +void HTMLBodyElement::setOnorientationchange(PassRefPtr<EventListener>
eventListener)
> +{
> +   
document()->setAttributeEventListener(eventNames().orientationchangeEvent,
eventListener);

This should be setWindowAttributeEventListener.

>
>  
> +#if ENABLE(ORIENTATION_EVENTS)
> +EventListener* HTMLFrameSetElement::onorientationchange() const
> +{
> +    return
document()->getWindowAttributeEventListener(eventNames().orientationchangeEvent
);
> +}
> +
> +void HTMLFrameSetElement::setOnorientationchange(PassRefPtr<EventListener>
eventListener)
> +{
> +   
document()->setAttributeEventListener(eventNames().orientationchangeEvent,
eventListener);

This should also be setWindowAttributeEventListener.

> +#if ENABLE(ORIENTATION_EVENTS)
> +	   void sendOrientationChangeEvent(int orientation);
> +	   int orientation() const;

These don't seem to be implemented anywhere.  Why should it be implemented per
platform? Shouldn't it just store the new orientation and dispatch the event?

r-