Re: [w3c/editing] Address VK spec review comments. (#317)

@yoavweiss commented on this pull request.

A few comments

> @@ -119,7 +118,7 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
                   The method must follow these steps:
                 </p>
                 <ol>
-                    <li>If the browsing context's <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-window">active window</a> does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.
+                    <li>If the <a href="https://heycam.github.io/webidl/#this">this's </a><a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a> is a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a> and does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.

The links could use [cross-references](https://respec.org/docs/#referencing-terms-from-other-specifications)

> @@ -131,7 +130,7 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
                       Call the system API to show the VK.
                   </li>
                   <li>
-                    When the VK is shown by the system, fire {{VirtualKeyboard/ongeometrychange()}} event.
+                    When the VK is shown by the system, the OS reports the bounding rectangle of the keyboard. Calculate the {{VirtualKeyboard/boundingRect}} which is the intersection of the keyboard rectangle reported by the OS and the layout viewport. Fire {{VirtualKeyboard/ongeometrychange()}} event to JS.

It'd be good to define what a "bounding rectangle" is. https://drafts.csswg.org/cssom-view/#dom-element-getboundingclientrect outlines an algorithm that calculates one. It'd be good to have a similar processing model here.

> @@ -143,7 +142,7 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
                   The method must follow these steps:
                 </p>
                 <ol>
-                  <li>If the browsing context is not focused, abort these steps.
+                    <li>If the <a href="https://heycam.github.io/webidl/#this">this's </a><a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a> is a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a> and does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.

Same comments as above. May be useful to pack this in a helper algorithm.

> @@ -167,15 +166,24 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
             </dt>
             <dd>
               <p>
-                When this attribute is set to `true`, a user agent MUST NOT resize its layout viewport or visual viewport.
+                A boolean, initially false. When this attribute is set to `true`, a user agent MUST NOT resize its layout viewport or visual viewport.
+              </p>
+              <p>
+                The {{VirtualKeyboard/overlaysContent}} getter steps are to return <a href="https://heycam.github.io/webidl/#this">this's </a>{{VirtualKeyboard/overlaysContent}}.

It'd be better to set and get a separate internal boolean variable.

> @@ -119,7 +118,7 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
                   The method must follow these steps:
                 </p>
                 <ol>
-                    <li>If the browsing context's <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-window">active window</a> does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.
+                    <li>If the <a href="https://heycam.github.io/webidl/#this">this's </a><a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a> is a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a> and does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.

"the" is not needed before `this`. The "'s" following `this` would be better outside the link.

> @@ -119,7 +118,7 @@ <h2>The <dfn>VirtualKeyboard</dfn> Interface</h2>
                   The method must follow these steps:
                 </p>
                 <ol>
-                    <li>If the browsing context's <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-window">active window</a> does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.
+                    <li>If the <a href="https://heycam.github.io/webidl/#this">this's </a><a href="https://html.spec.whatwg.org/#concept-relevant-global">relevant global object</a> is a <a href="https://html.spec.whatwg.org/multipage/window-object.html#window">Window object</a> and does not have <a href="https://html.spec.whatwg.org/multipage/interaction.html#sticky-activation">sticky user activation</a>, abort these steps.

Could this be called from a worker? If not, it's better to put this's relevant global in a variable, assert that it's a Window, and then abort if there's no user activation.

-- 
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/editing/pull/317#pullrequestreview-714312839