Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clarify Clients.matchAll() focus order for nested frames #1080

Closed
wanderview opened this issue Feb 23, 2017 · 8 comments · Fixed by #1259
Closed

clarify Clients.matchAll() focus order for nested frames #1080

wanderview opened this issue Feb 23, 2017 · 8 comments · Fixed by #1259

Comments

@wanderview
Copy link
Member

Clients.matchAll() says to order Clients so most recently focused clients are first in the list. What does this mean for nested iframes? Consider:

  1. A window with a nested iframe. The iframe is focused. In theory both the window and iframe Client.focused will be true. Will they also have the same focus time?
  2. A window that is already focused with a nested iframe that is not focused. The iframe is focused. Are the focus times for both windows updated or just the iframe?
@wanderview
Copy link
Member Author

Note, it may be difficult to write a WPT test to expose this behavior. At least in firefox we restrict what things like window.focus() based on a variety of heuristics. To write an effective test I'll like need to use a browser-specific test framework to get around these restrictions.

@wanderview
Copy link
Member Author

There is some discussion in IRC here where Jake and I conclude it should probably follow what browsers do for document.hasFocus():

http://logs.glob.uno/?c=freenode%23whatwg&s=23+Feb+2017&e=23+Feb+2017&h=focus#c1021602

This means parent/child frames get the same focus time when the child frame is focused. Although perhaps creation order is not the best there since ideally the child would be returned first, but current rules require window to be returned first (same time, creation order fallback).

@wanderview
Copy link
Member Author

wanderview commented Feb 28, 2017

Talking with @smaug----, it also seems a bit weird that the matchAll() steps link to this for the focus order:

https://html.spec.whatwg.org/multipage/interaction.html#focusing-steps

But those steps largely deal with browsing contexts. In contrast, Client objects represent environment or environment settings.

It seems we need something that tracks "last focus time" on the environment. This is basically what I am implementing, but it would be nice if the spec was clear about it.

@wanderview
Copy link
Member Author

Note, it may be difficult to write a WPT test to expose this behavior. At least in firefox we restrict what things like window.focus() based on a variety of heuristics. To write an effective test I'll like need to use a browser-specific test framework to get around these restrictions.

Ignore this. I just wrote the WPT test using iframes:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b53d88cb7099

The test codifies my best interpretation of the spec in its current state.

@smaug----
Copy link

(environment is basically Window, but the same Window can be reused for different Documents. Though, not sure if SW can be used with initial about:blanks )

@jakearchibald
Copy link
Contributor

jakearchibald commented Apr 3, 2017

F2F: Outside window appears first. Inner windows afterwards.

We think we can change this in future without too much breakage if we have to.

@xzhan96
Copy link

xzhan96 commented May 24, 2017

Hi @jakearchibald, this is related with Chromium issue 701233

Sorry, I didn't clear about the conclusion in chromium.

  1. A window with a nested iframe. The iframe is focused. In theory both the window and iframe Client.focused will be true. Will they also have the same focus time?
  2. A window that is already focused with a nested iframe that is not focused. The iframe is focused. Are the focus times for both windows updated or just the iframe?

jungkees added a commit that referenced this issue Jan 12, 2018
To reflect the decision to not expose reserved clients
(#1216) and to spec
.resultingClientId and .targetClientId in Nightly only, this:

* Removes
 - Client.reserved and its associated reserved state (V1, Nightly)
 - ClientQueryOptions.includeReserved (V1, Nightly)
 - FetchEvent.reservedClientId (V1)
 - FetchEvent.targetClientId (V1)
 - FetchEventInit.reservedClientId (V1)
 - FetchEventInit.targetClientId (V1)

* Changes
 - FetchEvent.reservedClientId to FetchEvent.resultingClientId (Nightly)
 - FetchEventInit.reservedClientId to FetchEventInit.resultingClientId
   (Nightly)
 - Handle Fetch to set FetchEvent.clientId for a navigation request to
   the empty string (V1)

* Corrects
 - matchedClients with clientObjects in Clients.matchAll() (V1, Nightly)

Related issue: #1245.

This also cleans up sort condition steps in Clients.matchAll() that
fixes #1080 (V1, Nightly)

(Changes for the Clients interface's methods will be addressed as
separate PRs.)
@jungkees
Copy link
Collaborator

Not sure if we are clear about this issue now. I haven't changed the behavior but tried to clean up the steps: https://github.com/w3c/ServiceWorker/pull/1259/files#diff-27b79860afe28f01aed4f1f6228367faR1232.

I think both Blink and Gecko implemented this introducing some "last focus time" info that's updated when focusing step runs on a frame in the document hierarchy. I think that conforms to what spec suggests, right?

But those steps largely deal with browsing contexts. In contrast, Client objects represent environment or environment settings.

I addressed this by defining and referencing the client's associated browsing contexts.

jungkees added a commit that referenced this issue Jan 13, 2018
To reflect the decision to not expose reserved clients
(#1216) and to spec
.resultingClientId and .targetClientId in Nightly only, this:

* Removes
 - Client.reserved and its associated reserved state (V1, Nightly)
 - ClientQueryOptions.includeReserved (V1, Nightly)
 - FetchEvent.reservedClientId (V1)
 - FetchEvent.targetClientId (V1)
 - FetchEventInit.reservedClientId (V1)
 - FetchEventInit.targetClientId (V1)

* Changes
 - FetchEvent.reservedClientId to FetchEvent.resultingClientId (Nightly)
 - FetchEventInit.reservedClientId to FetchEventInit.resultingClientId
   (Nightly)
 - Handle Fetch to set FetchEvent.clientId for a navigation request to
   the empty string (V1)

* Corrects
 - matchedClients with clientObjects in Clients.matchAll() (V1, Nightly)

Related issue: #1245.

This also cleans up sort condition steps in Clients.matchAll() that
fixes #1080 (V1, Nightly)

(Changes for the Clients interface's methods will be addressed as
separate PRs.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants