[webkit-reviews] review denied: [Bug 47301] AX: Support UIRequestEvents from new W3C Proposal : [Attachment 70115] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied chris fleizach <cfleizach at apple.com>'s
request for review:
Bug 47301: AX: Support UIRequestEvents from new W3C Proposal
https://bugs.webkit.org/show_bug.cgi?id=47301

Attachment 70115: Patch
https://bugs.webkit.org/attachment.cgi?id=70115&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70115&action=review

My major issues with this patch are (and maybe I am still missing the point):
- The events never seem to be fired.
- The undo/redo events seem to offer the same functionality as
http://www.whatwg.org/specs/web-apps/current-work/multipage/dnd.html#undomanage
revent from HTML5.
- These events seem general enough that they should be defined in HTML5 (or 6),
and not an accessibly spec.
- There should also be more tests that show the event getting fired in other
ways (other than manual dispatchEvent).

> WebCore/accessibility/UIRequestEvent.h:40
> +    enum {
> +	   
> +    };

Empty enum should be removed.

> WebCore/accessibility/UIRequestEvent.idl:38
> +    interface UIRequestEvent : UIEvent {
> +	   
> +	   void initUIRequestEvent(in DOMString type, 
> +				       in boolean canBubble, 
> +				       in boolean cancelable, 
> +				       in DOMWindow view, 
> +				       in long detail);
> +
> +    };

Perhaps this should be prefixed with webkit, until there is more support for
this standard.

> WebCore/dom/EventNames.h:176
> +    macro(undorequest) \
> +    macro(redorequest) \
> +    macro(escaperequest) \
> +    macro(deleterequest)

Again, these should probably be prefixed.