-
Notifications
You must be signed in to change notification settings - Fork 6
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
ScreenView focus() call can steal focus from the parent page #897
Comments
@emily-phet can you please state how you think we should proceed with this? A partner reported it, should we work out a patch that can be maintenance released? Should we first work towards fixing this bug on master? Now that we see that it is in result of a feature, and not a bug, will you comment on how you are thinking about this at this time? |
@jessegreenberg said:
Can you please comment on why we trigger focus on load? Is that conventional for websites to do? Is it a workaround for something? What would go wrong if we stopped triggering focus on load? UPDATE: @jessegreenberg already said:
I guess I still don't fully understand. Why do we have to trigger focus for the screen reader to start at the top? Could we do that only when you change screens, and not on startup? |
Yes! I think that would fix this, that is what I was trying to get at in my second sentence there. @zepumph also suggested
and that also seems like a good way to do this. This could be done fundamentally in scenery, like maybe our Removing my assignment for thoughts from @emily-phet, but this does seem like something we can/should fix. |
Not sure what logic we are talking about. If our only tool is document.activeElement, the most information we have is that when somewhere else outside the iframe is focused, our iframe's document.activeElement is the document.body. It is the body in a lot of other cases too. |
I started phetsims/scenery#1516 to brainstorm how to detect when the window has focus and think there may be overlap with phetsims/quadrilateral#311. |
Thanks @emily-phet for connecting me to this issue during our conversation today - I'm the member of the @PreTeXtBook community who reported this issue. I did just find https://stackoverflow.com/a/11969151#comment128118788_11969151 as a possible workaround on our end, but it looks like your team may be on track for a more elegant solution. Of course, if there's a more elegant way to embed these applets into a larger webpage/app besides an iframe, we might look into that as well. Thanks! |
@jessegreenberg are these failing PDOM unit tests from this change?
|
Yes, I think so. Though unit tests are passing for me locally on Chrome and Firefox. This is probably another case of phetsims/aqua#134. |
Thanks for pointing out @zepumph! A way to handle this for puppeteer was added above and scenery unit tests are passing again on CT. We should try to find a better way in phetsims/aqua#134. |
@StevenClontz thank you for reporting this and bringing it to our attention! We have fixed this and republished our sims so this does not happen anymore. This is no longer happening in the context you provided at https://pretextbook.org/examples/sample-article/annotated/section-interactive-server.html. Closing. |
Amazing. Thank you so much. |
@emily-phet just reported that this logic is causing trouble with a partner when trying to have phetsims load as one of many resources on a page. It will steal focus away from other parts of the parent frame. Here is the report:
It is easy to reproduce this with this script. iframes can steal their parent frame's focus:
parent:
iframe
The text was updated successfully, but these errors were encountered: