Skip to content

Commit

Permalink
Invoke the has focus steps instead of hasFocus() method directly. Fixes
Browse files Browse the repository at this point in the history 
#633.
  • Loading branch information
@jungkees
jungkees committed Nov 4, 2015
1 parent fbee35e commit 3d0ee14
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion spec/service_worker/index.html
View file
Expand Up @@ -3395,7 +3395,7 @@ <h1>Capture Window Client</h1>
<ol>
<li>Let <var>windowClient</var> be a new <code><a href="#window-client-interface">WindowClient</a></code> object that represents <var>browsingContext</var>.</li>
<li>Set <var>windowClient</var>'s <a href="#dfn-service-worker-client-visibilitystate">visibility state</a> to <var>browsingContext</var>'s <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-document">active document</a>'s <a href="http://www.w3.org/TR/page-visibility/#dom-document-visibilitystate">visibilityState</a> attribute value.</li>
<li>Set <var>windowClient</var>'s <a href="#dfn-service-worker-client-visibilitystate">focus state</a> to the result of running <var>browsingContext</var>'s <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-document">active document</a>'s <a href="https://html.spec.whatwg.org/multipage/interaction.html#dom-document-hasfocus">hasFocus()</a> method.</li>
<li>Set <var>windowClient</var>'s <a href="#dfn-service-worker-client-focusstate">focus state</a> to the result of running the <a href="https://html.spec.whatwg.org/multipage/interaction.html#has-focus-steps">has focus steps</a> with <var>browsingContext</var>'s <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-document">active document</a> as the argument.</li>

This comment has been minimized.

Sign in to view

Copy link
@annevk

annevk Nov 4, 2015

Member

Don't you need to queue a task for this?

This comment has been minimized.

Sign in to view

Copy link
@jungkees

jungkees Nov 4, 2015

Author Collaborator

The Capture Window Client algorithm is supposed to run in a task consumed by the client's responsible event loop. The callers invoking it are queuing a task already. A missing bit is I'm running it off the main thread in Clients.matchAll(). #672 tracks that.

This comment has been minimized.

Sign in to view

Copy link
@annevk

annevk Nov 4, 2015

Member

But then you're creating the WindowClient object in the wrong environment?

This comment has been minimized.

Sign in to view

Copy link
@jungkees

jungkees Nov 4, 2015

Author Collaborator

The algorithm takes the browsing context for which it creates the WindowClient.

This comment has been minimized.

Sign in to view

Copy link
@annevk

annevk Nov 4, 2015

Member

Right, but WindowClient is an object that only exists in service worker environments.

This comment has been minimized.

Sign in to view

Copy link
@jungkees

jungkees Nov 4, 2015

Author Collaborator

So, the capture operation is run in the client's event loop (in the queued task) but the captured WindowClient object is finally returned as the value the returned promise resolve with in the service worker environment. Does this seem wrong?

Once, back then, I asked you whether returning a promise in a thread and resolving a promise from a different thread (in a queued task) is fine, and that seemed okay.

I'm curious there would be a clearer way to spec it. If so, I'd like to refine it. Also, I'm getting my head around how to do the Capture Window Client in Clients.matchAll().

This comment has been minimized.

Sign in to view

Copy link
@annevk

annevk Nov 4, 2015

Member

Resolving it is sort of okay. But with non-primitive values it becomes broken. Not immediately sure what a better way to specify this would be. But assume you need to write the JavaScript code that creates all these objects and you can only use postMessage() to cross threads. That kind of gives you an idea as to what the constraints are.

This comment has been minimized.

Sign in to view

Copy link
@jungkees

jungkees Nov 6, 2015

Author Collaborator

I've tried to refine it: 09e47de.

In this patch, I no longer capture the window client's state (visibility state and focused) in the async algorithm steps. (I moved those steps to the queued task. <-- to be precise, the other methods except matchAll() had already set the states in the queued tasks.) And when the queued task has executed, the algorithm steps, where the task was queue from, create the Client/WindowClient object based on the captured states. Could you take a look?

This comment has been minimized.

Sign in to view

Copy link
@annevk

annevk Nov 6, 2015

Member

That is a rather massive change. Might be best to open an issue and assign it to me or some such if you want review.
<li>Return <var>windowClient</var>.</li>
</ol>
</spec-algorithm>
Expand Down
2 changes: 1 addition & 1 deletion spec/service_worker_1/index.html
View file
Expand Up @@ -3393,7 +3393,7 @@ <h1>Capture Window Client</h1>
<ol>
<li>Let <var>windowClient</var> be a new <code><a href="#window-client-interface">WindowClient</a></code> object that represents <var>browsingContext</var>.</li>
<li>Set <var>windowClient</var>'s <a href="#dfn-service-worker-client-visibilitystate">visibility state</a> to <var>browsingContext</var>'s <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-document">active document</a>'s <a href="http://www.w3.org/TR/page-visibility/#dom-document-visibilitystate">visibilityState</a> attribute value.</li>
<li>Set <var>windowClient</var>'s <a href="#dfn-service-worker-client-visibilitystate">focus state</a> to the result of running <var>browsingContext</var>'s <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-document">active document</a>'s <a href="https://html.spec.whatwg.org/multipage/interaction.html#dom-document-hasfocus">hasFocus()</a> method.</li>
<li>Set <var>windowClient</var>'s <a href="#dfn-service-worker-client-focusstate">focus state</a> to the result of running the <a href="https://html.spec.whatwg.org/multipage/interaction.html#has-focus-steps">has focus steps</a> with <var>browsingContext</var>'s <a href="https://html.spec.whatwg.org/multipage/browsers.html#active-document">active document</a> as the argument.</li>
<li>Return <var>windowClient</var>.</li>
</ol>
</spec-algorithm>
Expand Down

0 comments on commit 3d0ee14

Please sign in to comment.