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

Autofocus spec should probably allow browsers to delay running the focusing steps, maybe? #3551

Closed
bzbarsky opened this issue Mar 9, 2018 · 21 comments · Fixed by #4763
Closed
Labels
interop Implementations are not interoperable with each other topic: focus

Comments

@bzbarsky
Copy link
Contributor

bzbarsky commented Mar 9, 2018

Gecko recently made a change to not actually do autofocusing bits until after pending stylesheets have loaded, to prevent flashes of unstyled content.

It's not clear to me that the current spec language (which required queueing a task as soon as the autofocus element is inserted into the DOM) allows this behavior. Should it?

@annevk annevk added interop Implementations are not interoperable with each other topic: focus labels Mar 11, 2018
@domenic
Copy link
Member

domenic commented Mar 14, 2018

I agree the spec currently doesn't allow this behavior. I guess it's theoretically an observable interop problem to allow arbitrary UA-determined delays here. Is that worth it?

@bzbarsky
Copy link
Contributor Author

It doesn't need to be arbitrary, insofar as the concept of "rendering start" is specced and waits for sheets.

@domenic
Copy link
Member

domenic commented Mar 14, 2018

Fair point. I assume such a point is not specced today :-/.

@domenic
Copy link
Member

domenic commented May 9, 2019

@tkent-google @rniwa @cdumez do you know if Chrome or Safari do anything like this?

@domenic domenic mentioned this issue May 9, 2019
9 tasks
@tkent-google
Copy link
Contributor

It seems WebKit also has such delay.
Chrome doesn't have it for now. IMO the behavior of Firefox and WebKit is reasonable to avoid synchronous layout, and Chrome should follow it.

@annevk
Copy link
Member

annevk commented May 10, 2019

Could this be done as part of the "Rendering" section of the event loop? If a browser waits until it has various things before it runs that for the first time, it might be adequate?

Would it queue a task at that point to focus?

@domenic
Copy link
Member

domenic commented May 10, 2019

If we do it as part of update the rendering, I'd say we should do it as part of the list of things we do for each doc, so e.g. right before rAF callbacks perhaps.

Implementers, could you describe more precisely how you'd implement this kind of "holding"? Is update the rendering even reasonable---I kind of think that might happen before pending stylesheets have loaded?

An alternate approach would be to do it around the end of parsing, after

Spin the event loop until the first script in the list of scripts that will execute when the document has finished parsing has its "ready to be parser-executed" flag set and the parser's Document has no style sheet that is blocking scripts.

I'm unclear whether "stylesheets that are blocking scripts" are the same as the ones the OP is talking about.

@tkent-google
Copy link
Contributor

If we do it as part of update the rendering, I'd say we should do it as part of the list of things we do for each doc, so e.g. right before rAF callbacks perhaps.

I think change would be:

  • Not posting a task on the UI task source
  • Introduce something like "autofocus element queue" on every document. An element with autofocus is enqueued to it on inserting into a document.
  • Add a following step to update the rendering.
    14. For each fully active Document in docs, if document's script-blocking style sheet counter is 0, for each element in document's autofocus element queue, run the autofocus steps
    It should be later than "13. ... update the rendering or user interface of that Document" step because focusability check need style resolution in WebKit and Blink.

@bzbarsky Does this work well in Firefox?

WebKit's current behavior is something like doing the autofocus steps just after rendering completion without checking pending stylesheets.

@rniwa
Copy link

rniwa commented May 24, 2019

Making it possible to delay the autofocus makes sense to me. I'd have to think through as to what would be the ideal timing though.

@tkent-google
Copy link
Contributor

Correction: We should use only "autofocus element queue" of top-level browsing context's active document.

@tkent-google
Copy link
Contributor

ping @bzbarsky

@rniwa
Copy link

rniwa commented Jun 18, 2019

We should use only "autofocus element queue" of top-level browsing context's active document.

By that, you mean autofocus would only work in the top-level browsing context? Is that actually true in the existing browsers?

@tkent-google
Copy link
Contributor

By that, you mean autofocus would only work in the top-level browsing context? Is that actually true in the existing browsers?

No, I wanted to mean that elements with autofocus attribute in all sub-frames should be queued to the top-level queue in order to avoid race between frames.

@rniwa
Copy link

rniwa commented Jun 19, 2019

@tkent-google : That seems to suggest that autofocus is really page or chrome level feature so this queue perhaps should only exist in the top-level browsing context?

Somewhat relatedly, when you window.open and the focus moves to the newly opened auxiliary window, and the current window loses the focus, right? If the autofocus queues of the opener as well as the opened window each had an element in it, which one wins? In chronological order?

It seems that we'd need a lot of tests involving iframes, new auxiliary browsing contexts, and iframes opening auxiliary browsing contexts all interleaving autofocus element queue in one another, and observe what the browsers are doing today, and what we really want to spec.

@tkent-google
Copy link
Contributor

@rniwa Yes.

I don't think window.open will have issues. Auxiliary browsing context is a top-level browsing context, and page/tab focus state doesn't affect autofocus behavior.

Iframes are problematic. Suppose that a page has multiple iframes, and some of them contain elements with autofocus. I confirmed that the final focused element depended on iframe's load timing with Chrome, Firefox, and Safari. I don't have a good idea to make this deterministic. My current proposal doesn't affect it, and it's out of scope of this issue.

@rniwa
Copy link

rniwa commented Jun 25, 2019

I don't have a good idea to make this deterministic. My current proposal doesn't affect it, and it's out of scope of this issue.

While I don't think we necessarily need to make it deterministic, it's wholly within the scope of this issue to discuss how it affects them. While we worked on iPadOS, we found dozens of productivity websites that rely on focusing behavior across iframes. In particular, when about:blank iframe is inserted, autofocus behavior should be fully deterministic with wherever async model we spec.

@tkent-google
Copy link
Contributor

tkent-google commented Jun 25, 2019

@rniwa Hmm, probably I don't understand what's your concern precisely. Anyway, inserting about:blank iframes doesn't affect autofocus behavior, with the current specification's behavior, current major UA implementations, and my proposed change, AFAIK.

@rniwa
Copy link

rniwa commented Jun 25, 2019

@tkent-google : Consider the following markup:

<!DOCTYPE html>
<html>
<body>
<script>
const frame = document.createElement('iframe');
document.body.appendChild(frame);
frame.contentDocument.body.innerHTML = `<input autofocus>`;
</script>
<input autofocus>
</body>
</html>

In Firefox, the second input element is always focused. In Chrome, the first input element is always focused. In Safari, it changes each load LOL. There is no reason this needs to stay non-deterministic.

@tkent-google
Copy link
Contributor

@rniwa, thank you for the example.

With both of the current specification and my proposed change, the input-in-iframe should be focused because it is connected earlier than the input-in-top-document.

However, this example is not interoperable due to another reason. Chrome and Safari loads about:blank synchronously, and the innerHTML result remains. Firefox loads about:blank asynchronously, and the input-in-iframe is disconnected immediately.

If we change the script to:

const frame = document.createElement('iframe');
frame.onload = () => { frame.contentDocument.body.innerHTML = `<input autofocus>`; };
document.body.appendChild(frame);

Firefox focuses on the input-in-top-document in many cases, and I guess it's possible to focus on the input-in-iframe sometimes because iframe's load event can be dispatched before the input-in-top-document is connected.

@rniwa
Copy link

rniwa commented Jun 27, 2019

Hm... so it looks like Safari usually picks the last element with autofocus whereas Firefox & Chrome picks the first when there is no iframe involved.

Here's another interesting example. frame1 appears before frame2 but because the input field in frame1 is inserted after frame1 had finished loading, Chrome ends up focusing the input element in frame2. Firefox focuses the one in the main document due to iframe loading time, and Safari seems to focus a random element.

<!DOCTYPE html>
<html>
<body>
<script>
const frame1 = document.createElement('iframe');
document.body.appendChild(frame1);

const frame2 = document.createElement('iframe');
frame2.onload = () => {
    frame1.contentDocument.body.innerHTML = `<input autofocus>`;
    frame2.contentDocument.body.innerHTML = `<input autofocus>`;
}
document.body.appendChild(frame2);
</script>
<input autofocus>
</body>
</html>

So the autofocus queue somehow needs to close after each document had finished parsing / loading, and then remove itself from the top-level browsing context. There appears to be a lot of edge cases around here that worth careful examinations.

tkent-google added a commit to tkent-google/html that referenced this issue Jul 11, 2019
- Run focusing steps after an animation frame
- Don't autofocus if the top-level document has focused area
- Don't autofocus if one of ancestor document has :target element.

This fixes whatwg#3551 and whatwg#4645
@tkent-google
Copy link
Contributor

I posted a specification PR to run focusing steps after an animation frame: #4763

tkent-google added a commit to tkent-google/html that referenced this issue Sep 3, 2019
- Run focusing steps after an animation frame
- Don't autofocus if the top-level document has focused area
- Don't autofocus if one of ancestor document has :target element.

This fixes whatwg#3551 and whatwg#4645
domenic pushed a commit that referenced this issue Sep 3, 2019
* Run focusing steps after an animation frame
* Don't autofocus if the top-level document has focused area
* Don't autofocus if one of ancestor document has a :target element

Fixes #3551.  Fixes #4645.
zcorpan pushed a commit that referenced this issue Nov 6, 2019
* Run focusing steps after an animation frame
* Don't autofocus if the top-level document has focused area
* Don't autofocus if one of ancestor document has a :target element

Fixes #3551.  Fixes #4645.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: focus
Development

Successfully merging a pull request may close this issue.

5 participants