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

Fix exception when looking up indexedDB when disabled in Firefox. #798

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@Stebalien

Stebalien commented Jan 28, 2013

Firefox will sometimes throw an exception when looking up known but
disabled features in the dom. This commit catches exceptions when
looking up properties in testDOMProps.

While this fix could be more specific, putting it in the testDOMProps function will help prevent bugs like this from being introduced in the future.

Stebalien added some commits Jan 27, 2013

Catch exceptions when looking up properties in the DOM
Firefox will sometimes throw an exception when looking up known but
disabled features in the dom. This commit catches exceptions when
looking up properties in testDOMProps.
@paulirish

This comment has been minimized.

Member

paulirish commented Jan 28, 2013

Can you give me the repro here? Disable indexeddb in about:config and then run !!prefixed("indexedDB", window) ?

In the past we've been averse to try/catching just because someone "broke their warranty" in about:config.

@Stebalien

This comment has been minimized.

Stebalien commented Jan 28, 2013

Sorry, I should have been much clearer in my initial request. I'm working on a cross-browser extension/webapp that I'm currently porting it to Firefox (as an extension). Trying to access window.indexedDB from a Firefox extension throws an exception (same with window.localStorage, etc.).

I understand that Modernizr is not intended to be run inside extensions. When I initially encountered this problem, I mistakenly assumed that it applied to Firefox in general (like bug #92). As it doesn't, I understand if you feel this patch isn't altogether necessary.

@stucox

This comment has been minimized.

Member

stucox commented Jan 28, 2013

I think this is what's causing my Modernizr bookmarklet not to work in FF; same goes for a data: URL with a Modernizr build which includes indexeddb, e.g.:

data:text/html,<script src="https://gist.github.com/raw/4654472/f1597530cf8894d5729838f488cff4a62574dfd8/modernizr.custom.07390.js"></script>

Presumably Indexed DB is disabled in FF for bookmarklets (or javascript: URLs in general?) and data: URLs with the same mechanics as disabling it via about:config.

So maybe 3 developer/debug/hack use cases for sticking a try...catch in there.

@mgaert

This comment has been minimized.

mgaert commented Jan 28, 2013

@paulirish

This comment has been minimized.

Member

paulirish commented Feb 25, 2013

try/catch forces a slow path in JS engines. so i'd like to avoid this if neccessary.

especially for all everything. we could do a bypass for FF though.

@ghost ghost assigned stucox Feb 25, 2013

@stucox

This comment has been minimized.

Member

stucox commented Feb 25, 2013

Let's change it to an if (prop[i] in obj ) then.

I'll do this now and wrap up this PR.

@stucox stucox closed this in f258126 Feb 26, 2013

SlexAxton added a commit to SlexAxton/Modernizr that referenced this pull request Feb 27, 2013

Merge branch 'master' of github.com:Modernizr/Modernizr into meta
* 'master' of github.com:Modernizr/Modernizr:
  Remove `pre` from shiv version
  Remove `pre` from printshiv version
  Added detect for <template> HTML tag
  Update HTML5 Shiv to latest version
  Removed dashes from property names corresponding to feature detects.
  Changed an existance check to an `in` to avoid exceptions being thrown for disabled features in FF. Fixes Modernizr#798

@stucox stucox referenced this pull request Mar 2, 2014

Closed

v3.0 release notes #805

patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015

patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015

Merge branch 'master' of github.com:Modernizr/Modernizr into meta
* 'master' of github.com:Modernizr/Modernizr:
  Remove `pre` from shiv version
  Remove `pre` from printshiv version
  Added detect for <template> HTML tag
  Update HTML5 Shiv to latest version
  Removed dashes from property names corresponding to feature detects.
  Changed an existance check to an `in` to avoid exceptions being thrown for disabled features in FF. Fixes Modernizr#798
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment