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

Error: Assertion failed: The focused Node should have a one and only one PDOM instance LanguageSelectionNode#7268 #261

Open
zepumph opened this issue Jan 10, 2025 · 3 comments

Comments

@zepumph
Copy link
Member

zepumph commented Jan 10, 2025

Tagging @jessegreenberg as a potential locale/keyboard/preference bug. My primary goal for this issue is to silence this bug, but let me know if you have any ideas about solving this.

[PAGE ERROR] Error: Assertion failed: The focused Node should have a one and only one PDOM instance
 LanguageSelectionNode#7268

Right now it is showing up as this message, but actually this is downstream:

Error: Assertion failed: reentry detected, value=sr, oldValue=my
STACK: window.assertions.assertFunction<@http://127.0.0.1/continuous-testing/ct-snapshots/1736524986982/assert/js/assert.js:45:21
    at window.assertions.assertFunction< (http://127.0.0.1/continuous-testing/ct-snapshots/1736524986982/assert/js/assert.js:45:21)
    at assert (../../../../../axon/js/ReadOnlyProperty.ts:343:14)
    at _notifyListeners (../../../../../axon/js/ReadOnlyProperty.ts:291:13)
    at unguardedSet (../../../../../../joist/js/i18n/localeProperty.ts:62:10)
    at unguardedSet (../../../../../axon/js/ReadOnlyProperty.ts:271:11)
    at set (../../../../../axon/js/Property.ts:54:10)
    at localeProperty (../../../../../../joist/js/preferences/LanguageSelectionNode.ts:60:8)
    at listener (../../../../../axon/js/TinyEmitter.ts:213:6)
    at notifyLoop (../../../../../axon/js/TinyEmitter.ts:196:13)
    at emit (../../../../../axon/js/Emitter.ts:65:21)
    at emit (../../../../../../scenery/js/listeners/FireListener.ts:103:22)
    at fire (../../../../../../scenery/js/listeners/FireListener.ts:151:13)
    at callback (../../../../../../scenery/js/listeners/PressListener.ts:766:16)
    at apply (../../../../../tandem/js/PhetioAction.ts:162:16)
    at execute (../../../../../../scenery/js/listeners/PressListener.ts:519:24)
    at release (../../../../../../scenery/js/listeners/FireListener.ts:144:10)
    at release (../../../../../../scenery/js/listeners/PressListener.ts:865:11)
    at inputEvent (../../../../../../scenery/js/input/Input.ts:1875:91)
    at dispatchToListeners (../../../../../../scenery/js/input/Input.ts:1824:9)
    at dispatchEvent (../../../../../../scenery/js/input/Input.ts:1628:9)
    at upEvent (../../../../../../scenery/js/input/Input.ts:512:13)
    at apply (../../../../../tandem/js/PhetioAction.ts:162:16)
    at execute (../../../../../../scenery/js/input/Input.ts:1266:24)
    at touchEnd (../../../../../../scenery/js/input/InputFuzzer.js:242:24)
    at touchEnd (../../../../../../scenery/js/input/InputFuzzer.js:55:11)
    at action (../../../../../../scenery/js/input/InputFuzzer.js:107:6)
    at fuzzEvents (../../../../../joist/js/SimDisplay.ts:221:23)
    at fuzzInputEvents (../../../../../joist/js/Sim.ts:1146:42)
    at fuzzFrame (../../../../../joist/js/Sim.ts:1047:11)
@zepumph zepumph self-assigned this Jan 10, 2025
zepumph referenced this issue in phetsims/scenery Jan 10, 2025
@jessegreenberg jessegreenberg self-assigned this Jan 10, 2025
@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jan 10, 2025

Sorry, I took a look for a few minutes but didn't notice anything obvious. I couldn't get this to happen locally. I don't see the LanguageSelectionNode using DAG.

Using the debugger, the number of LanguageSelectionNode pdomInstances stays at 1 when the dialog is open. It does go to 0 when the dialog is closed.

I am wondering if something is breaking before the instance count is correct and this assertion is being thrown during a transient state.
Or maybe somehow code is being run after the dialog is closed.

Let me know if you want to take a look together.

@jessegreenberg jessegreenberg removed their assignment Jan 10, 2025
@zepumph
Copy link
Member Author

zepumph commented Jan 10, 2025

I don't see the LanguageSelectionNode using DAG.

I don't think it is about DAG, my guess is that it is about some sort of condition where the LanguageSelectionNode is the pdomFocusedNode but it doesn't have any pdomInstances during the closing of the Dialog. I too can't reproduce FYI. I'll keep investigating.

@zepumph
Copy link
Member Author

zepumph commented Jan 15, 2025

Ok, after thinking about this more, it seem that the main issue here is that we are making an assumption about the focused node: That is always has a pdomInstance and is in the PDOM.

I believe that this is not true, even if we want it to. It would make sense if we wanted to add an assertion to investigate that assumption a bit more. When a new focus is set. We add a listener to it that says "If you ever remove your pdomInstance, assert". Or "If you every remove your pdomInstance, blur/clear scenery focus."

No matter, the assertion we are hitting is downstream of this issue. We are in pdomInstance, and just wanting to make sure that when toggling to invisible, that we blur focus if applicable. One case where it isn't applicable is if the focus doesn't even have a pdomInstance, so I got rid of the assertion and just added grace. @jessegreenberg would you like me to investigate the underlying assumption about how we are sometimes having a FocusManager.pdomFocus of a Node that doesn't have a pdomInstance? Perhaps a different issue? Have you been working in this area at all?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants