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

Validate the ability to create an idb store #417

Merged
merged 1 commit into from
Aug 10, 2020
Merged

Conversation

trestletech
Copy link
Contributor

@trestletech trestletech commented Aug 7, 2020

The prior checks (line 1437 currently) test whether or not the browser supports the necessary features, but it doesn't actually exercise whether or not we're allowed to create a store here. Some browsers (such as Safari) have additional restrictions in e.g. cross-origin iframes that preclude us from successfully opening an idb store. This check ensures that we can actually create a store, or fails fast if not.

The error caused issues whether or not idb was ultimately used as the store for learnr state.

Without this, Safari is throwing the following error:

[Error] SecurityError: IDBFactory.open() called in an invalid security context
	open (idb-keyval-iife-compat.min.js:7:843)
	e (idb-keyval-iife-compat.min.js:7:843)
	(anonymous function) (tutorial.js:1518)
	(anonymous function) (tutorial.js:1705)
	c (jquery.min.js:2:28300)
	fireWith (jquery.min.js:2:29041)
	l (jquery.min.js:2:79812)
	(anonymous function) (jquery.min.js:2:82256)
[Error] SyntaxError: JSON Parse error: Unexpected identifier "object"
	r (chunk-vendors.a5db6293.js:83:31379)

Testing/Validation

This one's a doozy..

This PR better handles scenario where a feature we use to track the state of the user's learnr lesson called "IndexedDB" (IDB) is not supported. Previously, if a browser didn't support IDB we would silently fail and just throw away the user's learnr state at the end of the lesson. That's acceptable if the browser isn't able to store anything.

Unfortunately there's a bug in which learnr thought that IDB was supported in certain conditions but then when the browser goes to use it we only then see a failure. learnr didn't handle that gracefully. This PR fixes that.

There's a 2x2 matrix of scenarios that we care about:

IDB available IDB not available
custom storage Should work
IDB storage Should work, but no results saved

Prior to this PR, if IDB was not available you'd just get an error in the console and various features didn't work at all (e.g. multiple choice questions didn't render). As of this PR, the top right box should now work just fine. And the lower right box should work with the noted caveat that results aren't going to be saved.

(Note that the test app I setup for custom storage just uses a single file to store everyone's state. So if you had multiple users active on the app at the same time, they'd just be overwriting each other's state which would be confusing.)

The easiest way to simulate an environment where IDB is not available currently is to use Safari where the learnr doc is hosted inside a cross-origin iframe.

I went ahead and deployed a few of these for testing:

learnr v 9005 (master) learnr v 9006 (this PR)
Custom Storage custom-9005 custom-9006
IDB Storage idb-9005 idb-9006

In a browser like Firefox or Chrome, IDB is always available even with cross-origin iframes. But if you use Safari on those links, you'll have an environment where IDB fails.

  • If you visit the 9005 links in Safari, you should see them both fail (error in the console, state is not preserved across refreshes, some widgets don't render properly).
  • When you visit the 9006 links (this PR), you should see no JS console error and everything render properly. With custom-9006, you should see state preserved across refreshes (and even in between browsers). With idb-9006, all of the features should work except when you refresh the page, you'll start over at a blank app. Because we have no way to store learnr state so we silently gracefully fail.

All that to say:

  • Observe the failures on custom-9005 or idb-9005 on Safari and a JS error. The multiple choice question doesn't load properly. (Click the "Sample Learnr Widgets" section)
  • Ensure Safari on custom-9006 works and preserves state across refreshes
  • Ensure Safari on idb-9006 works other than starting over every time you refresh (no state storage)
  • Ensure Firefox/Chrome/Edge (?) work on custom-9006 and on idb-9006 -- including preserving state across refreshes on both.

The prior checks (line 1437 currently) test whether or not the browser supports the necessary features, but it doesn't actually exercise whether or not we're allowed to create a store here. Some browsers (such as Safari) have additional restrictions in e.g. cross-origin iframes that preclude us from successfully opening an idb store. This check ensures that we can actually create a store, or fails fast if not.

The error caused issues whether or not idb was ultimately used as the store for learnr state.

Without this, Safari is throwing the following error:

[Error] SecurityError: IDBFactory.open() called in an invalid security context
	open (idb-keyval-iife-compat.min.js:7:843)
	e (idb-keyval-iife-compat.min.js:7:843)
	(anonymous function) (tutorial.js:1518)
	(anonymous function) (tutorial.js:1705)
	c (jquery.min.js:2:28300)
	fireWith (jquery.min.js:2:29041)
	l (jquery.min.js:2:79812)
	(anonymous function) (jquery.min.js:2:82256)
[Error] SyntaxError: JSON Parse error: Unexpected identifier "object"
	r (chunk-vendors.a5db6293.js:83:31379)
@trestletech trestletech merged commit 38effb4 into master Aug 10, 2020
@trestletech trestletech deleted the idb-failure branch August 10, 2020 14:23
schloerke added a commit that referenced this pull request Aug 10, 2020
* master:
  Validate the ability to create an idb store (#417)
schloerke added a commit that referenced this pull request Sep 2, 2020
* master:
  Add events to website (#423)
  Add a checker test with error as solution (#422)
  Fixes for learnr on external evaluator (#420)
  Add shinytest support (#407)
  Validate the ability to create an idb store (#417)
  Disable completion of R code inside quotes, closes #401 (#413)
  Bump version (#414)
  Save tutorial output to temp folder when tutorial folder does not have write permissions (#412)
  Throw an informative error if an exercise chunk is NULL (#411)
  Remove outdated exercise option
  Checking for exercise errors (#403)
  Add event handler system (#398)
  External evaluator fixes (#399)
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