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

Sim won't behave properly after keyboard nav in full screen IE #317

Closed
KatieWoe opened this issue Oct 4, 2018 · 30 comments
Closed

Sim won't behave properly after keyboard nav in full screen IE #317

KatieWoe opened this issue Oct 4, 2018 · 30 comments
Assignees
Labels

Comments

@KatieWoe
Copy link
Contributor

KatieWoe commented Oct 4, 2018

Device
hp laptop
OS
Win 7
Browser
Internet explorer.
Problem Description
For phetsims/qa#200 possibly related to #314
When using keyboard nav in the full screen mode, if you switch to using a mouse the sim will become uninteractable. Any animations in progress (electron discharge) will continue, but mouse input will do nothing. Leaving full screen does not solve this. It seems the only way to leave is by pressing escape, as using keyboard nav does not seem to work. After escaping, if mouse was used in full screen the sim is still uninteractable with mouse. If it was not, and keyboard nav was used in an attempt to leave, the phet menu stays up, even when the sim is being manipulated.
Steps to Reproduce

  1. Go to the full screen
  2. Use keyboard nav to manipulate the sim
  3. Click something with the mouse and note how the sim responds
  4. Press escape to leave
  5. Hit refresh
  6. Repeate 1 and 2
  7. Use keyboard nav to try and leave full screen. Nothing happens
  8. Press escape to leave
  9. Note phet menu behavior
@jessegreenberg
Copy link
Contributor

@KatieWoe is this reproducible in Windows 10?

@KatieWoe
Copy link
Contributor Author

KatieWoe commented Oct 4, 2018

@jessegreenberg yes

@jessegreenberg
Copy link
Contributor

Good deal, that will make it easier to investigate. Thanks.

@jessegreenberg
Copy link
Contributor

Able to reproduce, shortly after the bug happened, IE11 crashed with "A problem occurred..." I verified that this is specific to IE11, and doesn't happen in Chrome or Edge.

@jessegreenberg
Copy link
Contributor

When I run a mangle=false version of the sim, IE11 tells me
SCRIPT16389: Incorrect function.

Pointing to this line

            blur: function() {
                this._primarySibling.blur();
            },

@jessegreenberg
Copy link
Contributor

Presumably this is in AccessiblePeer.

@KatieWoe
Copy link
Contributor Author

KatieWoe commented Oct 4, 2018

Noting here that this is present on Win 8.1 IE as well

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 4, 2018

The HTML element that we are trying to blur is defined in IE11.

UPDATE:
I removed the line this._primarySibling.blur();, and once I saw "Unspecified Error" instead. But now I can't reproduce.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 4, 2018

capture

I am pretty sure that what is happening is that blur() in IE11 throws an error if the element is not in the document.

EDIT: This doesn't explain why this isn't happening for other phet menu items.
EDIT: This doesn't seem to be the case, I added this

      if ( document.getElementById( this._primarySibling.id ) ) {
        this._primarySibling.blur();
      }

and the issue persists.

@jessegreenberg
Copy link
Contributor

Here are some modifications in a non-mangled built version:

blur: function() {
  window.peer = this, window.element = this._primarySibling, document.activeElement === this._primarySibling && this._primarySibling.blur();
},
  • window.element.blur() works just fine.
  • window.peer._primarySibling.blur() works just fine.
  • this._primarySibling.blur() is throwing an error "Incorrect Function".
  • I verified that window.peer._primarySibling === document.activeElement.

So it seems like things are OK in the console, but not OK at runtime.

@jessegreenberg
Copy link
Contributor

This bug report looks really similar to what we are seeing
https://bugs.jquery.com/ticket/15044

@jessegreenberg
Copy link
Contributor

Maybe the blur is a read herring, and the error is happening elsewhere.

@jessegreenberg
Copy link
Contributor

Some output in the console, but I still am very unclear on what is happening here
capture

@jessegreenberg
Copy link
Contributor

This isn't happening in the deployed version of the sim, that might help isolate the problem.

@KatieWoe
Copy link
Contributor Author

KatieWoe commented Oct 4, 2018

Just found that this issue is happening to an even larger extent in phetsims/qa#203 (Coulomb's Law). In addition to this issue, if keyboard nav is used on the home screen on full screen it crashes and looses many images.
odd

@jessegreenberg
Copy link
Contributor

Thanks @KatieWoe, that is the same issue. We are trying to blur document.activeElement in fullScreen mode and IE11 reports "Incorrect Function".

@jessegreenberg
Copy link
Contributor

So it seems like blur works as long as the element is not document.activeElement for IE11 in fullscreen mode...which means that blur doesn't work for IE11 in fullscreen mode. But this implementation of AccessiblePeer.blur makes the problem go away in both john-travoltage and coulombs-law:

    blur: function() {
      assert && assert( this._primarySibling, 'must have a primary sibling to blur' );

      var tempElement = document.createElement( 'button' );
      tempElement.focus();
      // this._primarySibling.blur();
    },

@jessegreenberg
Copy link
Contributor

A bug reported to jquery about this in https://bugs.jquery.com/ticket/15044 was closed with comment

This bug is esoteric enough that I will leave it for Microsoft. If it isn't fixed soon and becomes a commonly reported issue we could investigate a workaround.

But the sim becomes unusable in this case so Ill continue with #317 (comment) as a workaround.

@jessegreenberg
Copy link
Contributor

I tried addign this:

        if ( platform.ie11 && Fullscreen.isFullScreen() ) {
          IE_BLUR_ELEMENT.focus();
        }

and the sim works but document.activeElement still equals the primary sibling. Maybe this will work if IE_BLUR_ELEMENT is attached to the document?

@jessegreenberg
Copy link
Contributor

It did not work. #317 (comment) is a more graceful solution, and is definitely one way to go. Apparently IE11 has a special function setActive, maybe that will work better? http://help.dottoro.com/ljqmdirr.php

@jessegreenberg
Copy link
Contributor

It did not. Comments in other threads are suggesting a timeout might work as a workaround.

@jessegreenberg
Copy link
Contributor

A timeout (though ugly) seems to work great. All recomendations in this stackoverlfow suggest a timeout for focus issues in IE11
https://stackoverflow.com/questions/2600186/focus-doesnt-work-in-ie

@jessegreenberg
Copy link
Contributor

@zepumph helped me commit this change, we verified that this is working now in IE11 and in other platforms too. The release of JT will need a scenery branch with this commit.

@jessegreenberg
Copy link
Contributor

The above commit should fix all occurrences of this (JT, CL, GFL:B, and more). @KatieWoe can you please check that this is fixed in built versions of the sim in IE11 using phettest (since requirejs won't work in IE11)?

@KatieWoe
Copy link
Contributor Author

KatieWoe commented Oct 5, 2018

@jessegreenberg just test the three sims mentioned on Win 10 IE 11 and it all looks good.

jbphet pushed a commit to phetsims/scenery that referenced this issue Oct 8, 2018
@jbphet
Copy link
Contributor

jbphet commented Oct 8, 2018

This isn't working for me. I first tried propagating it to the RC branch, but the problem wasn't gone, so I tried building off master, and I'm still seeing the lockup there as well. I noticed the code is using this in a closure, which looks suspicious. Are you sure this is fixed?

@jessegreenberg
Copy link
Contributor

Sorry @jbphet, that looks very wrong. It is very weird that IE11 isn't breaking when @KatieWoe or I test it. I just tested it again off of master and behavior seems ok.

@jessegreenberg
Copy link
Contributor

I changed it to use self, but now that the reference to the AccessiblePeer is correct, the reference to _primarySibling is gone after the menu has been hidden, so self._primarySibling.blur(); is throwing an error for me now. I will have to follow up tomorrow.

@jessegreenberg
Copy link
Contributor

Worked with @jbphet to verify a fix. Removing my assignment for now.

@jessegreenberg jessegreenberg removed their assignment Oct 9, 2018
@KatieWoe
Copy link
Contributor Author

KatieWoe commented Oct 9, 2018

@jessegreenberg Looks ok on JT 1.5.0-rc.3 on Win 10 IE 11. Do you want this left open for a future maintenance release, or should it be closed?

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

No branches or pull requests

3 participants