Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Firefox Accounts renders blank-ish screen if dom.storage.enabled is false #1848

Closed
pdehaan opened this issue Nov 5, 2014 · 7 comments · Fixed by #1866
Closed

Firefox Accounts renders blank-ish screen if dom.storage.enabled is false #1848

pdehaan opened this issue Nov 5, 2014 · 7 comments · Fixed by #1866
Assignees

Comments

@pdehaan
Copy link
Contributor

pdehaan commented Nov 5, 2014

Found in https://accounts.stage.mozaws.net/ver.json (commit 310bd7d)

{
    "version": "0.25.0",
    "commit": "310bd7d9e6906dc6124cd536c33e4ee9bdd0ebc5",
    "l10n": "87cc8f9eb4",
    "tosPp": "a9ad0e4ed4"
}

Steps to reproduce:

  1. Launch Firefox.
  2. Go to about:config.
  3. Search for dom.storage.enabled and set it to false.
  4. Open a new tab and browse to https://accounts.stage.mozaws.net/
  5. Open the Dev Tools and switch to "Console"
  6. Weep uncontrollably.

Actual results:

firefox_accounts
Figure 1. Thats no good!

TypeError: t is null
— via 1da513a8.main.js:14

If I try and read the cryptic minimized content, I see the following (3) instances of t.:

  1. t.setItem(a,JSON.stringify(c)) (/app/scripts/vendor/crosstab.js:247?)
  2. var b=t.getItem(a) (/app/scripts/vendor/crosstab.js:256?)
  3. t.removeItem(u.keys[a]) (/app/scripts/vendor/crosstab.js:414?)

Expected results:

We should check for local/session storage and possibly fail fast(er) if dom.storage is disabled.
Thankfully it looks like all the errors are coming from vendor/crosstab.js, I think elsewhere the localStorage and sessionStorage calls are wrapped in try/catch blocks.

@ckarlof
Copy link
Contributor

ckarlof commented Nov 5, 2014

Yeah, we detect localStorage enabled in lib/config-loader.js, but the introduction of crosstab.js introduced this regression. @shane-tomlinson, take a look? We should also add metrics to measure how often localStorage and cookies are disabled.

/cc @kparlante

@shane-tomlinson
Copy link

Search for dom.storage.enabled and set it to false.

Tinfoilers...

@shane-tomlinson
Copy link

I'm trying to think if we require access to local/sessionStorage, or if FxA will work acceptably, albeit with a slightly degraded experience, with dom.storage.enabled set to false.

If we are unable to store data, the repercussions are:

  • No way to remember the user's previous session and user must sign in from scratch.
  • No way to pass session information from the verification tab and the password reset tab after the user completes the password reset.

IIUC, a Sync user with dom.storage.enabled set to false should be alright, the unwrapBKey and keyFetchToken are sent to the browser upon verification and should not be stored anyways.

@shane-tomlinson
Copy link

There are multiple problems when dom.storage.enabled is set to false.

  1. crosstab blows up on initialization.
  2. with crosstab completely disabled, reloading /cookies_disabled shows a blank screen.

@shane-tomlinson shane-tomlinson self-assigned this Nov 6, 2014
@jrgm
Copy link
Contributor

jrgm commented Nov 6, 2014

Search for dom.storage.enabled and set it to false.
Tinfoilers...

This came from https://bugzilla.mozilla.org/show_bug.cgi?id=1093915#c4, with the user reporting that they had a blank screen, and indicating that they had taken no action to set it to false intentionally.

@kparlante
Copy link

Yeah, we detect localStorage enabled in lib/config-loader.js, but the introduction of crosstab.js introduced this regression. @shane-tomlinson, take a look? We should also add metrics to measure how often localStorage and cookies are disabled.

screen.cookies_disabled and error.cookies_disabled error are relatively rare, and mostly occur on iPhones. Here's a week's worth: https://kibana.fxa.us-west-2.prod.mozaws.net/#dashboard/temp/AUmgR8wzemn6F9h2haem

If I'm reading the code correctly, this screen gets shown for either localStorage or cookies disabled:

areCookiesEnabled: function (force) {

@shane-tomlinson
Copy link

If I'm reading the code correctly, this screen gets shown for either localStorage or cookies disabled:

areCookiesEnabled: function (force) {

You are correct. @ryanfeeley and I talked about creating separate messages/screens for cookies and localStorage, but we felt the average user would have almost no clue what localStorage means. My intuition says the most common cause for a disabled localStorage is disabled cookies, but we do not have metrics around that and intuition is usually wrong.

We should make the metrics report for what caused the failure more specific.

shane-tomlinson pushed a commit that referenced this issue Nov 12, 2014
…ge is disabled.

* Work with the author of crosstab to ensure it works as expected with cookies and localStorage disabled.
* Ensure the /cookies_disabled page renders if the user reloads it while cookies are disabled.

fixes #1848
@pdehaan pdehaan added the has PR label Nov 13, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants