Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

getComputedStyle crashes on some pages #548

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

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

Collaborator

domenic commented Dec 20, 2012

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.

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

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

haraldrudell/jsdom-patched-for-node-0.11.4@21859d5

Collaborator

domenic commented Dec 26, 2012

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.

Collaborator

domenic commented Dec 27, 2012

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

Collaborator

domenic commented Dec 27, 2012

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.

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 in a72b95d Dec 29, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment