1801761 - UnsuppressAndInvalidate -> SetReadyForFocus may cause unexpected side affect when it's triggered by Element::Focus

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Sean Feng [:sefeng]]

While updating the autofocus algorithm, I noticed this assertion is flaky.

I think what happened was when Element::Focus run, we generated a new action id. And then FlushPendingNotifications was triggered by FlushAndCheckIfFocusable which might unsuppress the painting. If the parent document was not focused yet, SetReadyForFocus would focus the parent document by generating an new action id, and this new action made the previous action id staled.

Sounds plausible Emilio?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Sounds believable. Seems it should be easy to fix by generating the action id inside SetFocusInner rather than relying on the caller to provide it? Afaict most if not all callers just give a fresh focus action id.

Comment 2

[Sean Feng [:sefeng]]

Thanks, I'll fix this

Comment 3

[Sean Feng [:sefeng]]

Attachment: Bug 1801761 - Generate focus actionId in SetFocusInner rather than relying on the caller to provide it r=emilio

This fixes an issue where Element::Focus triggered another focus
event and caused the initial focus event failed to be executed.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:sefeng, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 5

[Sean Feng [:sefeng]]

I'll land this along with the patches for bug 1444491.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce06375518a7
Generate focus actionId in SetFocusInner rather than relying on the caller to provide it r=emilio,hsivonen

Comment 7

[Natalia Csoregi [:nataliaCs]]

Backed out along with Bug 1444491 for causing failures on autofocus-attribute.svg

• backout: https://hg.mozilla.org/integration/autoland/rev/7c9d11c84ac7a77507e4f8b7d136da56227e7ca3

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7da0a39b1461
Generate focus actionId in SetFocusInner rather than relying on the caller to provide it r=emilio,hsivonen

Comment 9

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/7da0a39b1461