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

Focus and blur events don't fire while fuzzing if the document does not have focus #134

Open
jessegreenberg opened this issue Mar 16, 2022 · 6 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

If the document does not have focus, focus and blur events don't fire. Code dependent on these listeners will never fire while the sim is in the background. For example, that means that FocusManager.pdomFocusProperty will always be null because it is set in response to focus and blur events.

This is causing phetsims/geometric-optics#384. I also ran into it this morning while working on KeyboardDragListener, seeing that none of interrupt from blur was never called while fuzzing which is a problem.

@jessegreenberg
Copy link
Contributor Author

@zepumph and I met to discuss. We tried to get a headless Chrome up and running to verify that focus and blur events do not fire on that platform and to find a workaround but we weren't able to get it running successfully. But we did find puppeteer/puppeteer#1462 which confirms that focus and blur won't fire in headless chrome/puppeteer.

We couldn't think of fix yet so we are going to add a check to only fuzz keyboard events if document.hasFocus() is true. That means that ?keyboardFuzz will only fuzz events if the sim is in the foreground and will do no fuzzing on CT.

We want to try again to get focus and blur events in headless chrome somehow so we can fuzz on CT again. If that is not possible we will need to hit the drawing board and maybe think of a scenery level abstraction to recreate focus and blur in this environment.

@jessegreenberg
Copy link
Contributor Author

I am seeing errors from fuzzBoard on CT still, I am curious how that is possible after the above commit. Perhaps my understanding of this issue is not correct.

Query: brand=phet&ea&fuzzBoard&voicingInitiallyEnabled&memoryLimit=1000
Uncaught Error: Assertion failed: there should be two pairs of equal opposite angles for a parallelogram
Error: Assertion failed: there should be two pairs of equal opposite angles for a parallelogram
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/assert/js/assert.js:25:13)
at QuadrilateralDescriber.getSecondDetailsStatement (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/quadrilateral/js/quadrilateral/view/QuadrilateralDescriber.js:435:19)
at QuadrilateralScreenView.getVoicingDetailsContent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.js:215:57)
at VoicingToolbarAlertManager.createDetailsContent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/joist/js/toolbar/VoicingToolbarAlertManager.js:42:23)
at LabelButtonRow.playContent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/joist/js/toolbar/VoicingToolbarItem.js:194:35)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/joist/js/toolbar/VoicingToolbarItem.js:88:13
at TinyProperty.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/TinyEmitter.js:68:9)
at BooleanProperty._notifyListeners (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/Property.js:227:23)
at BooleanProperty.set (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/Property.js:156:14)
at BooleanProperty.set value [as value] (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/Property.js:297:10)
id: Bayes Chrome
Snapshot from 3/31/2022, 6:05:07 AM

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented Apr 1, 2022

Perhaps we can just explicitly put focus on the frame where the sim is being fuzzed to ensure that it will produce focus and blur events. It seems like it is sometimes doing that but not always. Like https://github.com/phetsims/phet-io-wrappers/blob/17e332da4b14b43f1a54f7f00e87ffa278e05ec1/input-record-and-playback/inputRecordAndPlayback.js#L229

@jessegreenberg
Copy link
Contributor Author

This came up again in phetsims/joist#897. Since puppeteer doesn't support focus and blur events, unit tests for focus state were failing. Wrapping the tests in a document.hasFocus() check lets us skip the tests on puppeteer but it would be better to find a way to keep running these tests automated tests. IF that is impossible maybe a way to generalize the hasFocus check.

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented May 25, 2023

This is happening again on CT for a new set of focus tests:

scenery : top-level-unit-tests : unbuilt?ea&brand=phet-io
http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/scenery/scenery-tests.html?ea&brand=phet-io
393 out of 399 tests passed. 6 failed.
ParallelDOMTests: replaceChild failed:
f has focus before being replaced
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1143:10)

ParallelDOMTests: replaceChild failed:
testNode has focus after replacing focused node f
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1150:10)

ParallelDOMTests: replaceChild failed:
browser is focusing testNode
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1151:10)

ParallelDOMTests: replaceChild failed:
d has focus before being replaced
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1166:10)

ParallelDOMTests: swapVisibility failed:
c should now have focus after swapVisibility
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1442:10)

ParallelDOMTests: swapVisibility failed:
c should now have focus after swapVisibility
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1453:10)

ParallelDOMTests: focusable option failed:
focusable div should be focused after calling focus()
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1546:10)

ParallelDOMTests: move to front/move to back failed:
b should have focus after a moved to front
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1856:10)
...

I will opt out of these for now when the document is not active.

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented Jun 2, 2023

More tests have been failing. I added the opt out to many more tests. I looked for a more elegant way to opt out in a beforeEach or something but did not find better support from QUnit.

Now when I run scenery unit tests while the browser is in the background everything is passing. If this is ever resolved we should remove these opt-outs.

jessegreenberg added a commit to phetsims/scenery-phet that referenced this issue Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant