search

SocketCluster / socketcluster-client

JavaScript client for SocketCluster
MIT License
292 stars 91 forks source link

Safari private browsing breaks authToken localStorage implementation #46

Closed stipsan closed 8 years ago

stipsan commented 8 years ago

How to reproduce,

I have a personal project setup where this bug can be reproduced in a few steps:

  1. open a new private tab in safari
  2. go to https://epic.vg
  3. type "Batman" and submit the form
  4. nothing happens, web inspector will show you an error that goes like: "QuotaExceededError: DOM Exception 22: An attempt was made to add something to storage that exceeded the quota."

    Sample fix

If we update this: https://github.com/SocketCluster/socketcluster-client/blob/a65b40bf8cd8327a3184d9b9e4883b292c7b5900/lib/auth.js#L9 With something like this: https://gist.github.com/philfreo/68ea3cd980d72383c951 The problem is solved.

I didn't have the time to fork and submit a PR with the fix, but thought I'd share this and my findings and submit a PR later if nobody beats me to it :smile:

jondubois commented 8 years ago

@stipsan A PR would be welcome since I don't have Safari to test (I'm using Ubuntu). I guess we should add the new lines underneath https://github.com/SocketCluster/socketcluster-client/blob/a65b40bf8cd8327a3184d9b9e4883b292c7b5900/lib/auth.js#L9 (still inside the same try catch block). We should avoid setting a generic key name like 'localStorage' (we don't want to overwrite anything that might be in there from before) - Maybe something like:

localStorage.setItem('__scLocalStorageTest', 1);
localStorage.removeItem('__scLocalStorageTest');
stipsan commented 8 years ago

Sure no problem! I'll submit a PR in a few hours