Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

getComputedStyle crashes on some pages #548

Closed
wants to merge 1 commit into from

4 participants

@dambalah

It happens because we are calling split on undefined on line 252. This fix checks ruleSet.selectorText before calling split on it.

@domenic
Collaborator

Oh nice, great catch. Can you add a test that fails before this change, and passes after it? Let me know if you need help figuring out where to put that/how to write it.

@haraldrudell

I had this, too.
Here's my fix:

selectors = typeof ruleSet.selectorText == 'string' ? ruleSet.selectorText.split(/\s*,\s*/) : [];

haraldrudell@21859d5

@domenic
Collaborator

Ah, thanks for the reminder. It'd be awesome if either of you put together a proper pull request with tests, otherwise I'll try to get around to this soon.

@domenic
Collaborator

Actually, it shouldn't be possible for selectorText to be a non-string. @chad3814, any ideas?

@domenic
Collaborator

Yeah, I can't find any way to reproduce this locally guys. @dambalah, @haraldrudell if you could give me the pages where this was breaking I'd be super-grateful.

@larryaubstore

I send you a branch with a unit test and the patch of @haraldrudell. The test will fail without the patch of @haraldrudell.

@domenic domenic closed this pull request from a commit
@domenic domenic Fix #548: crashes calling `getComputedStyle` when @media rules are in…
… stylesheet.

See also #549.
a72b95d
@domenic domenic closed this in a72b95d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 20, 2012
  1. @dambalah
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  lib/jsdom/browser/index.js
View
2  lib/jsdom/browser/index.js
@@ -249,7 +249,7 @@ exports.createWindow = function(dom, options) {
forEach.call(node.ownerDocument.styleSheets, function (sheet) {
forEach.call(sheet.cssRules, function (ruleSet) {
- selectors = ruleSet.selectorText.split(/\s*,\s*/);
+ selectors = ruleSet.selectorText ? ruleSet.selectorText.split(/\s*,\s*/) : [];
matched = false;
selectors.forEach(function (selectorText) {
if (!matched && matchesDontThrow(node, selectorText)) {
Something went wrong with that request. Please try again.