Bug 88799

Summary: Add new Pointer Lock spec attribute webkitPointerLockElement.
Product: WebKit Reporter: Vincent Scheib <scheib>
Component: New BugsAssignee: Vincent Scheib <scheib>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, ojan, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 88810    
Bug Blocks: 84402    
Attachments:
Description Flags
Patch
none
Patch
none
Patch dglazkov: review+

Description Vincent Scheib 2012-06-11 13:28:29 PDT
Add new Pointer Lock spec attribute webkitPointerLockElement.
Comment 1 Vincent Scheib 2012-06-11 13:38:06 PDT
Created attachment 146895 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2012-06-11 13:54:09 PDT
Comment on attachment 146895 [details]
Patch

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
Comment 3 Vincent Scheib 2012-06-11 14:36:09 PDT
Created attachment 146915 [details]
Patch
Comment 4 Vincent Scheib 2012-06-11 16:00:54 PDT
Created attachment 146947 [details]
Patch
Comment 5 Vincent Scheib 2012-06-11 16:02:38 PDT
Comment on attachment 146895 [details]
Patch

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?

Removed.

>> 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

Done. Changed to use Conditional=POINTER_LOCK, V8EnabledAtRuntime=pointerLock.

>> 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.

Done.

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

Done.
Comment 6 Vincent Scheib 2012-06-11 20:02:25 PDT
Committed r120031: <http://trac.webkit.org/changeset/120031>