Flushing of autofocus candidates should happen before updating the rendering of documents

[rniwa]

#4763 added the step 14 to flush autofocus candidates after the step 13 to update the rendering or user interface of that Document and its browsing context. This has two issues:

• We've already marked the painting time in step 12. It doesn't make much senes to include the focusing step's runtime in the painting time.
• A newly focused element by flushing autofocus candidates should have a browser's focus ring by default. Drawing of this focus ring will be delayed until the next rendering opportunity as it currently stands.

[rniwa]

@domenic @smaug---- @tkent-google @annevk @bzbarsky

[tkent-google]

The flush was put after step 13 because it requires computed style to determine focusability, and we'd like to avoid additional style computation by the flush.

If the flush is put before step 12, it triggers style computation for focusability check, and trigger style computation again for :focus style in step 13.

I don't have a strong opinion on the current order. It's ok to move it before step 12.

[rniwa]

That sounds like more of an engine optimization issue. You don’t have to update layout or style in your engine at any given point since that’s really not supposed to be observable. What’s important is when things get painted to the screen because that’s observable. From that perspective, we need to update the autofocus before we update the painting / rendering of the page.

[tkent-google]

How about moving the flush between 10 'run the animation frame callbacks' and 11 'run the update intersection observations steps' because the flush can change the scroll position?

[rniwa]

If the argument is that flushing could change the scroll position, then perhaps we need to be flushing it before scroll event get dispatched in step 6. It would be silly if we just fired scroll events, focused the element to scroll, then delayed scroll events until the next frame.

[tkent-google]

perhaps we need to be flushing it before scroll event get dispatched in step 6.

That's reasonable.
I'd like to put the flush just before 'run the resize steps' because it's difficult for Blink to do something between 'run the resize steps' and 'run the scroll steps'.

[rniwa]

perhaps we need to be flushing it before scroll event get dispatched in step 6.

That's reasonable.
I'd like to put the flush just before 'run the resize steps' because it's difficult for Blink to do something between 'run the resize steps' and 'run the scroll steps'.

That sounds reasonable to me assuming that the scroll position for the newly focused element is computed using the new window size and what not.

[tkent-google]

Specification PR: #5015