[webkit-reviews] review denied: [Bug 68468] WIP: Implement Mouse Lock API : [Attachment 108394] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Vincent Scheib
<scheib at chromium.org>'s request for review:
Bug 68468: WIP: Implement Mouse Lock API
https://bugs.webkit.org/show_bug.cgi?id=68468

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108394&action=review


Chromium WebKit API changes all look kosher to me!

> Source/WebCore/dom/Document.idl:249
> +	   FIXME: Need to sort out how to make this compile:

It might be better to separate the Document* changes out into a separate
patch since there isn't much here anyways and it has this signature issue
to still resolve.

> Source/WebCore/dom/MouseEvent.idl:29
> +	   readonly attribute long	       movementX;

I'd probably want to vendor prefix these attributes until they are finalized.

> Source/WebCore/dom/MouseEvent.idl:49
> +					      in
[Optional=CallWithDefaultValue] long movementX,

ditto

> Source/WebCore/dom/WheelEvent.h:54
> +#if ENABLE(MOUSE_LOCK_API)

It might be nicer to avoid these #ifdefs and just unconditionally give these
internal
data structures the extra fields.  However, that would imply expanding these
structures
for ports that haven't enabled this feature yet.  I don't know how much that
matters.
I sort of prefer the cleanliness of less conditionally compiled code :)