-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add option to Element.focus() to specify async application #6077
Comments
Why can't the page simply schedule a callback with |
|
Focus kinda needs two style updates somehow doesn't it? You need to know the styles pre-setting focus to know if the focus target is indeed focusable, and if it is, then focus can change styles itself (or styles of arbitrary elements) via the |
Not to say that applying focus async is not desirable of course, just that some kind of extra work seems mostly unavoidable. Gecko and I think WebKit too have fast paths to try to avoid updating the style too much for the "determine whether something is focusable" case. |
I agree that two style updates are needed: one to clean before running the algorithm to determine which element receives focus, and one to apply the focus. And as you say, browsers can and do optimize for this second style update, so a double render is indeed avoided. Note also that in the problematic situation leading to me filing this issue, it went like this:
If focus were run async, it'd be:
This runs one full style recalc and one partial style recalc, whereas the sync version runs two full style recalcs. |
That's an interesting point. Is it sensible to specify that the async focus call is ignored if the target is not focusable when resolved? This would lead to the following problem: If you async focus A..N and N is not focusable, what do you do? Do you drop prior focus requests A..M? Do you keep a list of focus requests A..N? Do you walk through that list forward or backwards until you find a valid target? |
I'm not sure I follow. I don't think the sync version would be particularly more expensive that the async one, for two reasons:
I guess in the case where the script tasks in (1) and (3) dirty intersecting set of styles, iff you don't have something like the WebKit optimization above, you might compute styles for a couple more elements than needed, but that's the only case that comes to mind... Maybe that's what you meant though?
What does "when resolved" mean? You mean when the browser actually goes through the async focus requests? If so, yeah, I imagine that's a way it could work (I think that's how Unless you mean that you check whether the element is focusable without updating style/layout? If so, I think that'd be bad, as that'd make stuff like: let textarea = document.createElement("textarea");
document.body.appendChild(textarea);
textarea.asyncFocus(); Work or not depending on whether something updates the style of the page before |
Yes, browsers do have such optimizations. The cost of the extra style recalc will depend on how much was invalidated. In general it is true that browsers can and do optimize to reduce the extra costs, but there is still a cost.
I agree that the extra style recalc to apply the style of the focused element and other elements that depend on focus state might be expensive. But that won't be made any worse with this async proposal, no?
Are you referring here to the style recalc done to apply styles affected by presence of focus? I.e. the "partial style recalc" I mentioned in step 4. |
No, not particularly.
No, I mean something a bit different. So, if the two script tasks don't touch intersecting parts of the DOM, there shouldn't be much perf difference from recalculating style in between via sync The case where you'd waste work (and thus where asyncFocus() helps) is if they touch the same parts of the DOM, right? Where you'd style them twice, once as the style recalc However, you could also build the opposite example, where the script task in (3) touches the DOM that That is, I'm still having a bit of a hard time to find good examples where this helps, due to
So whether this is worth depends on whether the cases this improves are more common than the cases it regresses IMO (with the usual cost of adding a new APIs etc factored in of course). |
Correct. In that scenario, most of the time there would not be much difference in performance.
Right.
Why would there be any restyling before asyncFocus is called? In your example, the performance would be about the same with asyncFocus as with focus. I think this is a special case of the case you mentioned above.
If you're thinking about aggregate improvement in average page interaction speed, or even expected improvements for many sites, I would agree with you. However, consider it from the page author's perspective: if you want to guarantee that your site has no performance problems of this kind, and asyncFocus was not available, then you have to do a bunch of special and fragile work to polyfill it. Stepping back to the larger picture though: on a case-by-case basis your arguments can make some sense. After all, asyncFocus will only fix a tiny fraction of the total source of jank on the web, and adding it does have a cost. But architecturally, it makes sense for us to make progress towards a platform that has no forced style recalcs or layouts at all if used correctly, and one that is not hard to use correctly. This goal will allow sites to achieve reliable and understandable performance for their script tasks (*), and allow user agents maximum ability to optimize rendering speed on their behalf. For these reasons, we should add asyncFocus. (*) Otherwise they will somehow have to account for script task timing measurements that sometimes have significant browser work inside them. |
The current spec for Element.focus specifies (like other HTML APIs) that focus applies immediately, so that for example a call to
document.activeElement
reflects the new focused element.While this is easy to understand and program to, it has the downside that if a developer calls
focus()
, the browser has no choice but to force-compute styles for all elements, or at the very least the elements that might be affected by thefocus()
call. This is bad because it can lead to style thrashing in code like this:without an intervening update-the-rendering event loop task, because styles will in general have to be computed twice. (Sites these days have complex rendering machinery using frameworks, and it's often not feasible for them to make sure to change styles at just the right times to avoid this style thrashing.)
I propose to add an
async
option toFocusOptions
. The behavior would be to run the focus algorithm steps at step 13 of update-the-rendering (which allows it to be placed after style recalc is done in a browser implementation).focus()
is commonly called on sites in order to improve accessibility during and after single-page-app transitions; facebook.com for example currently does this. @bgirard.The text was updated successfully, but these errors were encountered: