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

some sims are briefly unresponsive after computer has been idle (Safari) #140

Closed
pixelzoom opened this issue Jul 22, 2014 · 42 comments
Closed

Comments

@pixelzoom
Copy link
Contributor

Affected platform: Mac OS with Safari 7 or 8-beta

To reproduce:

  1. run a sim
  2. interact with the sim
  3. don't touch your computer for at least 3 minutes
  4. resume interacting with the sim

When you do step 4, the sim will be unresponsive for a few seconds. This is a concern because students may not touch their computer for a few minutes while listening to the teacher. And when they resume work with the sim, this interrupts the activity and is disconcerting.

This was first reported in phetsims/graphing-lines#42, but has been demonstrated with other sims. It occurs on Mac OS with Safari 7 and 8-beta. It does NOT occur with all sims, or with Safari on iOS, or with Chrome or Firefox on Mac OS.

See the above issue for details on discussion and prior investigation.

@jbphet
Copy link
Contributor

jbphet commented Jul 24, 2014

Assigned to @jonathanolson for testing on Scenery 0.2. If this doesn't resolve it, he should assign to @samreid to explore workarounds.

@pixelzoom
Copy link
Contributor Author

@pixelzoom will try this on a desktop Mac, in case this is somehow related to laptop power management.

@samreid
Copy link
Member

samreid commented Jul 26, 2014

I reproduced the problem on my Macbook Air 10.9.4 + Safari 7.0.5, while it was plugged in, using Graphing Lines (master). No problems appear in Energy Skate Park: Basics.

@samreid
Copy link
Member

samreid commented Jul 26, 2014

I tested times for the configuration above (my Macbook Air 10.9.4 + Safari 7.0.5, while it was plugged in, using Graphing Lines (master)) and found the following data points:
30 sec no problems
45 sec no problems
55 sec big problems
60 sec big problems
180 sec big problems

@samreid
Copy link
Member

samreid commented Jul 26, 2014

I tested this with scenery/ohtwo branch (on my configuration otherwise as above) and the problem is still there. May be good for @jonathanolson to investigate, but I'm going to look into this as well. Reassigned to @samreid for now.

@samreid samreid assigned samreid and unassigned jonathanolson Jul 26, 2014
@samreid
Copy link
Member

samreid commented Jul 26, 2014

With the Web Inspector open but without profiling or recording timeline, the problem goes away.

@samreid
Copy link
Member

samreid commented Jul 26, 2014

If I enable the profiler with a keystroke (Shift+option+command), it "truncates" the long pause time.

@samreid
Copy link
Member

samreid commented Jul 26, 2014

In scenery/master if I set all of the batchDOMEvents to false everywhere (6 occurrences or so), the problem goes away. So the problem seems closely related to scenery input event batching. I'll reassign to @jonathanolson for discussion.

@samreid samreid assigned jonathanolson and unassigned samreid Jul 26, 2014
@samreid
Copy link
Member

samreid commented Jul 26, 2014

I added this line after an event is added to the batch events callbacks:

console.log( input.batchedCallbacks.length );

During normal sim usage, this number is between 1-4 or so. During the error occurrence, I saw this in the console.

[...]
[Log] 97 (Input.js, line 671)
[Log] 98 (Input.js, line 671)
[Log] 99 (Input.js, line 671)
[Log] 100 (Input.js, line 671)
[Log] 101 (Input.js, line 671)
[Log] 102 (Input.js, line 671)
[Log] 103 (Input.js, line 671)
[Log] 104 (Input.js, line 671)
[Log] 105 (Input.js, line 671)
[Log] 106 (Input.js, line 671)
[Log] 107 (Input.js, line 671)

This seems related to the problem.

@samreid
Copy link
Member

samreid commented Jul 26, 2014

I published a gist here that shows the event as a function of time:
https://gist.github.com/samreid/72a5a145cae30ea0480f

When the user tries to interact with the sim (in this case around 111 sec) the addBatchedEvent starts adding up. Then around 113 sec they stop adding up and there is a 5 second pause. At 118 sec it invokes all of the callbacks.

@samreid
Copy link
Member

samreid commented Sep 11, 2014

Just a reminder that we could try some sort of "heartbeat" workaround to keep the sim alive when it is not in use.

@jessegreenberg
Copy link
Contributor

Reopening. There is a bug with VoiceOver in Safari that causes the virtual cursor to move to the heartbeat div when the innerHTML changes. Found in phetsims/john-travoltage#235.

Adding hidden to the div seems to fix the problem. But maybe the heartbeat div is not necessary any more. I will try removing the div and will see if the problem is still there on PhET's supported iOS/macOS devices.

@jessegreenberg jessegreenberg reopened this May 1, 2017
@jessegreenberg
Copy link
Contributor

jessegreenberg commented May 1, 2017

Supported Apple platforms as of 5/1/17

  • iOS 9.3.5 + Safari
  • iOS 10.0.1 + Safari
  • MacOS 10.9.5 + Safari
  • MacOS 10.10.5 + Safari
  • MacOS 10.11.6 + Safari
  • MacOS 10.12 + Safari

A check means it was tested without the heartbeat div and did not exhibit this issue.

jessegreenberg added a commit that referenced this issue May 1, 2017
@jessegreenberg
Copy link
Contributor

Thanks @samreid. @KatieWoe initially reported this again on Mac 10.14.5 Safari 12.1.1 and @jbphet saw this on OSX version 10.10.5, Safari version 10.1.2

@jessegreenberg
Copy link
Contributor

@samreid suggested taking a look at the developer tool "timeline" in Safari to get any clues as to where time is being spent, @KatieWoe if you have time for this we could try to get that info together.

@KatieWoe
Copy link

KatieWoe commented Jun 5, 2019

Was not able to reproduce on blackbody spectrum. Either the rc or on master.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 5, 2019

@KatieWoe and I met to try get information out of the developer tool "timeline" but we could not see the bug when the developer tools were open. Here is a summary in order of what we tried, all tests were with blackbody-spectrum off of master, using Jordan 10.14 partition.

  • We tried without a11y and did not see the problem
  • We tried with a11y and did see the problem
  • We tried again with a11y again and did see the problem
  • We tried with a11y while dev tools were recording and did not see the problem
  • We tried again with a11y while dev tools were recording and did not see the problem
  • We then tried with a11y without the dev tools open and did not see the problem
  • We tried with a11y after reloading the sim and did see the problem, but the delay was only about 1 second
  • We tried with a11y while using the "timeline" after reloading the sim and did not see the problem

After the first three tests, we thought that that this indicated a11y was the cause. But the following tests make this seem random.

EDIT: Since this shows up unpredictably, I tried again several times to reproduce this issue on Dirac with ohms-law, with and without a11y, with and without dev tools open. I couldn't get it to happen.

@jessegreenberg
Copy link
Contributor

Adding developer meeting label in case anyone else has ideas for what to try next.

Also assigning to @ariel-phet so he is aware of this issue and to confirm that this is a priority that needs to be worked on further.

@jonathanolson
Copy link
Contributor

jonathanolson commented Jun 5, 2019

Does the "classic" style heartbeat div fix the issue? (Not that we would go back to it). There could be something visual that Scenery changes instead of a div changing.

@ariel-phet ariel-phet removed their assignment Jun 6, 2019
@jbphet
Copy link
Contributor

jbphet commented Jun 6, 2019

This sounds a bit like the delays that we were seeing in phetsims/scenery#621. @samreid will check to see if this is related.

@jbphet
Copy link
Contributor

jbphet commented Jun 6, 2019

@jessegreenberg - @jonathanolson and I have a couple of suggestions above, and we discussed in dev meeting and no other immediate ideas came up. Back to you for now.

@samreid
Copy link
Member

samreid commented Jun 6, 2019

I set to integrated graphics card and was still not able to reproduce the problem, even when waiting 2 minutes between interactions.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 14, 2019

Thanks for reviewing all. I am able to see this issue on a macOS Mojave 10.14.5 at https://phet-dev.colorado.edu/html/ohms-law/1.4.0-rc.3/phet/ohms-law_en_phet.html. I will try the "classic" heartbeat div recommended in #140 (comment). I think that means removing aria-hidden and adding it to the body rather than the display div.

UPDATE: I tried moving the div to the body and removing aria-hidden and the bug still happens.

@jessegreenberg
Copy link
Contributor

I am able to see this very consistently with ?a11y enabled and I am never able to see this without it, so I am pretty convinced this is caused by a11y somehow. I confirmed that even with a11y enabled the problem does not happen when recording with the "Timelines" Safari tool.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 14, 2019

I found two ways to fix this issue in AccessibleSiblingStyle.js: Add z-index: 1 to the PDOM root, or position elements of the PDOM with absolute rather than fixed. Both changes work with desktop and mobile screen readers. I don't understand what is happening here yet. Putting z-index: -1 on the heartbeat div does not solve the problem, which indicates it is not an issue of layering. I tried making the root of the PDOM absolutely positioned and the sibling elements fixed and I still saw the bug.

fixed does not seem like the correct portioning attribute for AccessiblePeer siblings anyway, so I am going to make the change to positioning rather than use the z-index fix.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 14, 2019

I asked @KatieWoe to try to produce the bug with this version with the fix and she could not: https://phet-dev.colorado.edu/html/ohms-law/1.5.0-dev.3/phet/ohms-law_en_phet.html. This issue is fixed again, closing.

This was caused by relatively recent changes to AccessibleSiblingStyle so we are confident that no sims have been published with a11y and this issue.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 16, 2019

Reopening. phetsims/scenery#981 identifies why 'fixed' positioning is important for a11y, PDOM elements are positioned relative to the viewport. So we either need to go with the z-index workaround or change the way PDOM elements are positioned.

I am going with the z-index solution for now, I don't think it is worth changing our approach since this workaround is available and reasonable. Change made in the following commit, I will verify once more tomorrow that this does actually fix the issue.

@jessegreenberg
Copy link
Contributor

I verified several times on macOS 10.14.5 that this is fixed.

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

7 participants