Re: [w3c/pointerlock] Add lock options to requestPointerLock (#49)

@JamesHollyer commented on this pull request.

A few big mistakes in the last version. Hopefully this one looks better.

> -            mouse events to other elements. Events that require the concept of
-            a mouse cursor must not be dispatched (for example: `mouseover`,
-            `mouseout`, `drag`, and `drop`).
-          </p>
-          <p>
-            In the locked state the system mouse cursor must be hidden.
-            Movement and button presses of the mouse must not cause the window
-            to lose focus.
-          </p>
-          <p>
-            Synthetic mouse events created by application script act the same
-            regardless of lock state.
+            Perform the following steps:
+            <ol>
+              <li>
+                Let <dfn>promise</dfn> be <a href="https://heycam.github.io/webidl/#a-new-promise">a new promise</a>.</li>

Changed.

> +                <ol>
+                  1. <a href="https://html.spec.whatwg.org/multipage/webappapis.html#queue-an-element-task">
+                  Queue an element task</a> on the 
+                  <a href="https://html.spec.whatwg.org/multipage/webappapis.html#user-interaction-task-source">
+                  user interaction task source</a>, given this, to perform the following steps:
+                  <ol>
+                    <li>
+                      <a href=https://dom.spec.whatwg.org/#concept-event-fire">Fire an event</a> named
+                      <a>pointerlockerror</a> at <a>this</a>'s 
+                      <a href="https://dom.spec.whatwg.org/#concept-node-document">node document</a>.
+                    </li>
+                    <li>
+                      <a>Reject</a> <a>promise</a> with a "{{WrongDocumentError}}" {{DOMException}}.
+                    </li>
+                    <li>
+                      Return <a>promise</a>.

Makes sense. I think I fixed it everywhere.

> -            to lose focus.
-          </p>
-          <p>
-            Synthetic mouse events created by application script act the same
-            regardless of lock state.
+            Perform the following steps:
+            <ol>
+              <li>
+                Let <dfn>promise</dfn> be <a href="https://heycam.github.io/webidl/#a-new-promise">a new promise</a>.</li>
+              </li>
+              <li>
+                If the <a href="https://heycam.github.io/webidl/#this">this</a>'s
+                <a>shadow-including root</a> is the <a>active document</a> of a
+                [=Document/browsing context=] which is (or has an <a>ancestor
+                browsing context</a> which is) in focus by a window which is in
+                focus by the operating system's window manager:

YUP!

> +                      <a>Reject</a> <a>promise</a> with a "{{InvalidStateError}}" {{DOMException}}.
+                    </li>
+                    <li>
+                      Return <a>promise</a>.
+                    </li>
+                  </ol>
+                </ol>
+              </li>
+              <li>
+                If the user agent's <a>pointer-lock target</a>'s <a>shadow-including
+                root</a> is equal to <a>this</a>'s <a>shadow-including root</a>, then:
+                <ol>
+                  <li>
+                    If <a>options</a> are equivalent to the current <a>pointer-lock options</a> then the <a>pointer-lock target</a>
+                    must be updated to this element, a <a>pointerlockchange</a>
+                    event sent, and the promise must <a>resolve</a>.

Changed. I linked to a webIDL definition of this.  Is that sufficient? 

> +                  <li>
+                    If <a>options</a> are not equivalent to the user agent's current
+                    <a>pointer-lock options</a> and <a>options</a> are not supported:
+                    <ol>
+                      1. <a href="https://html.spec.whatwg.org/multipage/webappapis.html#queue-an-element-task">
+                      Queue an element task</a> on the 
+                      <a href="https://html.spec.whatwg.org/multipage/webappapis.html#user-interaction-task-source">
+                      user interaction task source</a>, given this, to perform the following steps:
+                      <ol type='i'>
+                        <li>
+                          <a href=https://dom.spec.whatwg.org/#concept-event-fire">Fire an event</a> named
+                          <a>pointerlockerror</a> at <a>this</a>'s 
+                          <a href="https://dom.spec.whatwg.org/#concept-node-document">node document</a>.
+                        </li>
+                        <li>
+                          <a href="https://html.spec.whatwg.org/multipage/webappapis.html#queue-an-element-task">

Yea I saw that while workin on updating the return statements. Fixed it.

> +                    <ol>
+                      <li>
+                        Update the user agent's <a>pointer-lock target</a> to this element.
+                      </li>
+                      <li>
+                        Update the user agent's <a>pointer-lock options</a> to <a>options</a>
+                      </li>
+                      <li>
+                        <a href="https://html.spec.whatwg.org/multipage/webappapis.html#queue-an-element-task">
+                          Queue an element task</a> on the 
+                          <a href="https://html.spec.whatwg.org/multipage/webappapis.html#user-interaction-task-source">
+                          User Interaction task source</a>, given this, to perform the following steps:
+                          <ol type='i'>
+                            <li>
+                              <a href=https://dom.spec.whatwg.org/#concept-event-fire">Fire an event</a> named
+                              <a>pointerlockerror</a> at <a>this</a>'s 

haha Nope. I fixed this when going through all the return statements too. Good catch.

> +                            </li>
+                          </ol>
+                      </li>
+                    </ol>
+                  </li>
+                </ol>
+              </li>
+              <li>
+                If the user agent's current pointer-lock target's shadow-including root is the
+                same as this element's shadow-including root, then:
+                <ol type='i'>
+                  <li>
+                    The user agent's <a>pointer-lock target</a> must be updated to this element
+                  </li>
+                  <li>
+                    a <a>pointerlockchange</a> event must be sent

Yup fixed this too.

> +                  </li>
+                </ol>
+              </li>
+              <li>
+                If the user agent's current pointer-lock target's shadow-including root is the
+                same as this element's shadow-including root, then:
+                <ol type='i'>
+                  <li>
+                    The user agent's <a>pointer-lock target</a> must be updated to this element
+                  </li>
+                  <li>
+                    a <a>pointerlockchange</a> event must be sent
+                  </li>
+                  <li>
+                    the promise must <a>resolve</a>
+                  </li>

Yup.

> +                If the user agent's current pointer-lock target's shadow-including root is the
+                same as this element's shadow-including root, then:
+                <ol type='i'>
+                  <li>
+                    The user agent's <a>pointer-lock target</a> must be updated to this element
+                  </li>
+                  <li>
+                    a <a>pointerlockchange</a> event must be sent
+                  </li>
+                  <li>
+                    the promise must <a>resolve</a>
+                  </li>
+                </ol>
+              </li>
+              <li>
+                If the request was not started from an <a>engagement gesture</a> and 

That is correct if we already have a PointerLock Target the user gesture is not required.

> +                      <a href="https://dom.spec.whatwg.org/#concept-node-document">node document</a>.
+                    </li>
+                    <li>
+                      <a>Reject</a> <a>promise</a> with a "{{SecurityError}}" {{DOMException}}.
+                    </li>
+                    <li>
+                      Return <a>promise</a>.
+                    </li>
+                  </ol>
+                </ol>
+              </li>
+              <li>
+                <dl data-dfn-for="PointerLockOptions" data-link-for=
+                "PointerLockOptions">
+                  If <a>options</a>["<a>unadjustedMovement</a>"] is true and the platform does not
+                  support <a>unadjustedMovement</a>:

Step 4 is what to do if Pointer Lock is already acquired and this request is just to change it. 4.2 is where the Pointer Lock is acquired and the new request does not allow the options. This Step is for a brand new request that is not supported.

> +                          <a>pointerlockerror</a> at <a>this</a>'s 
+                          <a href="https://dom.spec.whatwg.org/#concept-node-document">node document</a>.
+                        </li>
+                        <li>
+                          <a href="https://html.spec.whatwg.org/multipage/webappapis.html#queue-an-element-task">
+                            Queue an element task</a> on the 
+                            <a href="https://html.spec.whatwg.org/multipage/webappapis.html#user-interaction-task-source">
+                            User Interaction task source</a>, given this, to perform the following steps:
+                            <ol type='i'>
+                              <li>
+                                <a href=https://dom.spec.whatwg.org/#concept-event-fire">Fire an event</a> named
+                                <a>pointerlockerror</a> at <a>this</a>'s 
+                                <a href="https://dom.spec.whatwg.org/#concept-node-document">node document</a>.
+                              </li>
+                              <li>
+                                The <a>promise</a> must be resolved.

Oh I messed myself up here. It was decided that in this situation it would be best to not fire an event at all and reject the promise. If you are attempting to changing to unsupported options you are obviously using this new API so you should not need an error event. Some developers had issue with an error event here because error events to them meant they did not have pointer lock. Here they still do have pointer lock. So reject is right but the error event is wrong.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/pointerlock/pull/49#pullrequestreview-532740713

Received on Wednesday, 18 November 2020 06:21:09 UTC