Skip to content

Fix timer not starting bug#53

Merged
lgarron merged 1 commit into
cubing:masterfrom
JoshHumpherys:master
Nov 20, 2015
Merged

Fix timer not starting bug#53
lgarron merged 1 commit into
cubing:masterfrom
JoshHumpherys:master

Conversation

@JoshHumpherys
Copy link
Copy Markdown
Contributor

Spacebar wouldn't start timer in Google Chrome. Only certain keys fire keypress event, and different browsers have different standards. All keys fire keydown event.

@lgarron
Copy link
Copy Markdown
Member

lgarron commented Nov 20, 2015

Spacebar wouldn't start timer in Google Chrome.

This is working for me in Chrome. I'd like it to work for everyone, but I'd also like to confirm whether this is really a problem before fixing. Could you let me know:

  1. Do you have anything on the page selected?
  2. What is your OS?

@JoshHumpherys
Copy link
Copy Markdown
Contributor Author

I had only tested on Windows 10 in Chrome, Edge, Firefox and IE. I only got it to fail in Chrome.
I just did more tests on various operating systems:
Windows 10[Chrome, Edge, Firefox, IE], Ubuntu[Chrome, Chromium, Firefox], Windows 7[Chrome, Firefox, IE], OS X 10.9.5[Chrome, Safari]
The only test that failed was Chrome on Windows 10, but it only failed on 2 of the 3 computers I tried it on. One of the computers that passed and the computer that failed were both running Chrome version 46.0.2490.86 (Official Build) m (32-bit) with Javascript V8 4.6.85.31. The specs were identical. (I don't know the version of the other Windows 10 computer that failed).

And yes, on both the computers that failed I tried clicking on pretty much everything. Nothing worked.

On one of the Windows 10 computers that failed I ran
window.addEventListener("keypress",function(){console.log("keypress");});
which worked for all letters, numbers, punctuation, and the enter key, but not the spacebar.

When I ran
window.addEventListener("keydown",function(){console.log("keydown");});
almost every key on the keyboard started firing the event, but most importantly, the spacebar began working as well.

This fixed the problem for me:
document.body.addEventListener("keydown",timerApp._timerController._keyDown.bind(timerApp._timerController));
So I changed the original code from keypress to keydown.

One last thing to note, offline version worked fine. It didn't work when running from timer.cubing.net but when I forked, cloned, and ran it locally the spacebar began working again. This made it hard for me to test, but I believe the fix should work just fine (except it will allow keys such as Control, Alt, and Shift to start the timer, when they didn't start it before).

@JoshHumpherys
Copy link
Copy Markdown
Contributor Author

I just tried Chrome on 2 more Windows 10 computers and a Windows 8.1 computer. All of them worked. All running Chrome 46.0.2490.86.

@lgarron lgarron merged commit 48266e6 into cubing:master Nov 20, 2015
@lgarron
Copy link
Copy Markdown
Member

lgarron commented Nov 20, 2015

I can't reproduce this in Chrome on Windows 10 (in a VM), but keydown seems to be the better choice, so I might as well merge.

And yes, on both the computers that failed I tried clicking on pretty much everything.

Clicking with the mouse or tapping a keyboard key?

One last thing to note, offline version worked fine.

Could you clarify what you mean by this?

  • Running offline in the browser (using the versions cached at timer.cubing.net).
  • Running index.html from the filesystem.
  • Running from a local webserver.

This made it hard for me to test, but I believe the fix should work just fine (except it will allow keys such as Control, Alt, and Shift to start the timer, when they didn't start it before).

Have you tested this? I explicitly make sure that only spacebar starts the timer, and the Ctrl/Alt/Shift don't seem to stop the timer for me (one of my annoyances with qqTimer is that it stops if you switch windows/tabs).

@JoshHumpherys
Copy link
Copy Markdown
Contributor Author

Clicking with the mouse or tapping a keyboard key?

I tried clicking with the mouse and tabbing around to get the focus on different elements.

Could you clarify what you mean by this?

  • Running offline in the browser (using the versions cached at timer.cubing.net).
  • Running index.html from the filesystem.
  • Running from a local webserver.

Yes, I forked and cloned your repo and ran index.html from the filesystem.

This made it hard for me to test, but I believe the fix should work just fine (except it will allow keys such as Control, Alt, and Shift to start the timer, when they didn't start it before).

Have you tested this?

Yes, but I meant to say that these keys would stop the timer, not start. Only spacebar will start the timer.

By the way, thank you. The timer works wonderfully now.

@lgarron
Copy link
Copy Markdown
Member

lgarron commented Nov 20, 2015

Cool, I think things are mostly working as expected; thanks for the fix!

Yes, I forked and cloned your repo and ran index.html from the filesystem.

Hmm, I don't think that should make a difference, but I've seen weirder browser behaviour.

Yes, but I meant to say that these keys would stop the timer, not start.

Oha, not for me. I'll file a bug for this.

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

Successfully merging this pull request may close these issues.

2 participants