[webkit-reviews] review denied: [Bug 88799] Add new Pointer Lock spec attribute webkitPointerLockElement. : [Attachment 146895] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Vincent Scheib
<scheib at chromium.org>'s request for review:
Bug 88799: Add new Pointer Lock spec attribute webkitPointerLockElement.
https://bugs.webkit.org/show_bug.cgi?id=88799

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=146895&action=review


> Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:104
> +    static bool webkitPointerLockElementEnabled() { return
isPointerLockEnabled; }

Why is this necessary?

> Source/WebCore/dom/Document.idl:261
> +	   readonly attribute [V8EnabledAtRuntime] Element
webkitPointerLockElement;

This should just be [V8EnabledAtRuntime=POINTER_LOCK]. No need for extra
defines: http://trac.webkit.org/wiki/WebKitIDL#V8EnabledAtRuntime

> Source/WebCore/page/PointerLockController.h:49
> +    Element* element() const { return m_element.get(); }

Please don't do inline them like this. If you want this inlined, put the body
of the function in this header file, after class declaration.

> LayoutTests/pointer-lock/pointer-lock-api.html:9
> +    description("Basic API existance test for Pointer Lock.")

existance -> existence