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

timeoutDialog timer does not evaluate time elapsed #987

Open
nubz opened this issue Aug 8, 2018 · 3 comments
Open

timeoutDialog timer does not evaluate time elapsed #987

nubz opened this issue Aug 8, 2018 · 3 comments

Comments

@nubz
Copy link

nubz commented Aug 8, 2018

As the basis for the dialog timer in timeoutDialog.js is a fixed number of milliseconds from the initialisation of the timer then when the tab is not in focus (or "sleeping") the timer runs extremely slowly (or not at all) thereby destroying the intention of it.

Problem/Feature

When a loaded page initialises a timeout dialog to warn users of imminent signing out and when I focus on another tab for an amount of time equal to the configured time out and then return to the timer tab the actual time elapsed is not taken into account at all and the user has been signed out without any warning. This can be observed in any version of Chrome since 2011. https://developers.google.com/web/updates/2017/03/background_tabs

Expected behaviour

When I return to the timer tab after an amount of time equal to or greater than the configured timeout then I expect the handler for automatic signing out to be triggered and if I return within the countdown period then I expect the dialog handler to be triggered.

Proposed solution

Change the method used for the timer from setTimeout to setInterval and regularly check the absolute time elapsed. If within the countdown period then clear the interval and trigger the dialog and if time has exceeded the timeout then trigger the handler for signOut. For example instead of the existing setUpDialogTimer() we could replace it with something like:

var timeoutInterval

function expired() {
  return getDateNow() >= settings.signout_time
}

function withinCountdown() {
  return !expired() && getDateNow() >=  settings.signout_time - (settings.countdown * 1000)
}

function checkTimeElapsed() {
  if (expired()) {
    signOut()
  } else if (withinCountdown()) {
    window.clearInterval(timeoutInterval)
    setupDialog()
  }
}

function setupDialogTimer() {
  settings.signout_time = getDateNow() + settings.timeout * 1000
  timeoutInterval = window.setInterval(checkTimeElapsed, 1000)
}

Explain the steps to reproduce the bug

Use Chrome to load a page with a timeoutDialog running and then focus on another tab until the configured timeout has elapsed. Then return to the tab running the timer and it should be evident that the timer has not kept up with real time.

Tell us about your environment

  • Version used: AF 3.2.3 (I know this is not the latest but looking at the latest refactored code the same pattern is still in use and my example above is based on current release 3.3.1)
  • Browser Name and version: Chrome Version 67.0.3396.99 (Official Build) (64-bit)
  • Operating System and version (desktop or mobile): Laptop Ubuntu 16.04
@nataliecarey
Copy link
Contributor

Hi @nubz, one of the changes in the new implementation of the timeout dialog was to remove the expectation that setInterval will be called exactly once every 1000 milliseconds. There's a test for it here https://github.com/hmrc/assets-frontend/blob/master/assets/patterns/help-users-when-we-time-them-out-of-a-service/timeout-dialog.test.js#L525 and the implementation is here https://github.com/hmrc/assets-frontend/blob/master/assets/patterns/help-users-when-we-time-them-out-of-a-service/timeoutDialog.js#L101

Have you tested with the latest version of AF? I'd expect the upgrade to solve your problem, if not I'll gladly look deeper into it.

@nataliecarey
Copy link
Contributor

In conversation with @gordonmcmullan I can see the logic of the problem. I'll take a look.

@nubz
Copy link
Author

nubz commented Aug 9, 2018

Hi @matcarey - thanks for looking and yes when I see the tests around this all explicitly fail if a setInterval is being used... these would need quite an overhaul to implement something like this, and to be clear the problem is explicitly about relying on a forward moving timer to move forward in real time and this does not happen when the tab is not in focus. Both setTimeout and setInterval are unreliable versus real time... using setInterval at least gives us a chance to evaluate time elapsed.

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

2 participants