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

Workaround for IE throwing InvalidAccessError on db open. Fixes #32 #34

Closed
wants to merge 1 commit into from

Conversation

jonohill
Copy link

Despite typeof dbVersion equalling "number" in IE, it throws an InvalidAccessError when attempting to call open. parseInt still results in dbVersion being a number, as you'd expect, but seems to remind IE of what it's getting.

@@ -102,7 +102,8 @@ angular.module('indexedDB', []).provider '$indexedDB', ->

createDatabaseConnection = ->
deferred = $q.defer()
dbReq = indexedDB.open(dbName, dbVersion or 1)
# IE11 workaround - parseInt reminds IE that dbVersion is actually a number
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line exceeds maximum allowed length

@jonohill
Copy link
Author

I've been doing a bit more research on this one. The real issue actually stems from the behaviour of Math.max in the upgradeDatabase function - if passing a single argument it appears the result is not integery enough - Number.isInteger returns true but indexedDB.open fails. Passing two or more args to Math.max makes everything fine.

I found a bug report at Microsoft Connect which covers the Math.max issue but it 404s to I've made a new one covering it and the interaction with indexedDB.open. I wouldn't hold out too much hope as the old bug report was filed in 2011.

As for what to do here, there are at least a few options, such as:

  • Use parseInt with .open, per this pull request.
  • Move parseInt to the source of the issue, i.e. dbVersion = parseInt(Math.max.apply(...)
  • Skip Math.max if there's only one version

I'm thinking option 3. Thoughts?

As for making this IE specific, I can't think of a nice way to test for the bad behaviour. The result of Math.max looks like an integer in all the tests I have tried, and testing specifically for IE is increasingly messy, especially with MS Edge which tries its hardest to look like Chrome.

@bramski
Copy link
Owner

bramski commented May 28, 2015

Option 3 sounds just fine. Make sure you generate the source files and run the tests before you resubmit a PR.

@jonohill
Copy link
Author

jonohill commented Jun 7, 2015

Closing this PR. See #38 with the new fix.

@jonohill jonohill closed this Jun 7, 2015
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.

3 participants