Bug 88799 - Add new Pointer Lock spec attribute webkitPointerLockElement.
Summary: Add new Pointer Lock spec attribute webkitPointerLockElement.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vincent Scheib
URL:
Keywords:
Depends on: 88810
Blocks: 84402
  Show dependency treegraph
 
Reported: 2012-06-11 13:28 PDT by Vincent Scheib
Modified: 2012-06-11 20:02 PDT (History)
5 users (show)

See Also:


Attachments
Patch (17.66 KB, patch)
2012-06-11 13:38 PDT, Vincent Scheib
no flags Details | Formatted Diff | Diff
Patch (18.31 KB, patch)
2012-06-11 14:36 PDT, Vincent Scheib
no flags Details | Formatted Diff | Diff
Patch (18.27 KB, patch)
2012-06-11 16:00 PDT, Vincent Scheib
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>