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

Catch against IE's broken storage events #47

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dpogue
Copy link
Contributor

@dpogue dpogue commented May 17, 2014

Only deal with storage events that come while the document is not in focus.
See also: http://stackoverflow.com/questions/18476564/ie-localstorage-event-misfired

This caused us some problems when we were making multiple writes to localStorage and seeing data get overwritten with old values or disappear entirely:

  1. Assign stuff to $localStorage.
  2. $localStorage debounce happens and writes it to browser localStorage.
  3. Assign more stuff to $localStorage.
  4. IE fires to storage event, we update $localStorage and overwrite our changes from step 3.
  5. $localStorage debounce happens and detects no changes because they were overwritten.

Only deal with storage events that come while the document is not in focus.

See also: http://stackoverflow.com/questions/18476564/ie-localstorage-event-misfired
@futurechan
Copy link

@dpogue can you update your branch? I've had issues with IE as well.

@dpogue
Copy link
Contributor Author

dpogue commented Jul 21, 2015

@futurechan I'm not sure I want some of the newer changes, particularly ones that will trigger extra digest cycles and slow Angular down :(

@futurechan
Copy link

@dpogue do you plan to maintain your fork indefinitely?

@futurechan
Copy link

I wonder what recourse I have. This has been clutch for firefox and chrome users.

@egilkh
Copy link
Contributor

egilkh commented Jul 21, 2015

I think the best (and should maybe fix IE event handling) would be to have ngStorage's $localStorage and ngStorage's event handling use the same debounce for updating it's values. As it is now when handling storageEvents it's kind of hacky (and it does force an apply, extra digest cycle).

I'll look into that if you want me to.

@futurechan
Copy link

@egilkh that would be great. I'm willing to try too, but I can tell you right now I have no idea what I'm doing.

@abeninskibede
Copy link

Great fix! The master branch is not working at all in IE 10/ IE11 for me

@egilkh
Copy link
Contributor

egilkh commented Jul 30, 2015

@abeninskibede Nice to know you think it's great!

Can you provide a error message or explanation of what is it that is not working?

@abeninskibede
Copy link

@egilkh Don't see any errors in the console.

The bug:
when I add items to an array created from $localstarage.default in IE 95% time items are are not persisted in local storage.

@egilkh
Copy link
Contributor

egilkh commented Jul 30, 2015

@abeninskibede

Hmm, that is indeed strange. I must admit I almost never use $default. Would you be able to provide a failing snippet of code (minimal example) so I can dig into it (easier than me guessing your usage) ?

@abeninskibede
Copy link

    var defaultTabs = [
        { name: 'tab 1',  isSelected: true, widgets: [] },
        { name: 'tab 2',  isSelected: false, widgets: []}
    ];
    var tabs = $localStorage.$default({ tabs: defaultTabs }).tabs;

Then I push widgets using tabs[x].widgets.push(new {})

egilkh added a commit to egilkh/ngStorage that referenced this pull request Oct 1, 2015
Should fix gsklee#171, gsklee#167
Ref gsklee#47

Not sure if gsklee#47 is fixed yet.
@isuda isuda deleted the patch-2 branch March 8, 2016 17:47
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.

5 participants