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

jsdom/level2/style.js: Fixed ReferenceError in the scanForImportRules helper function. #348

Merged
merged 1 commit into from Oct 31, 2011

Conversation

Projects
None yet
3 participants
@papandreou
Copy link
Contributor

commented Oct 25, 2011

No description provided.

@elfsternberg

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2011

Description: scanForImportRules has a call to return the sheet passed in, but the calling code in evaluateStyleSheet() passes only the cssRules. This results in breakage.

@papandreou

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2011

Sorry for not providing more details about the actual error. I started getting a bunch of these when manipulating certain documents:

ReferenceError: self is not defined
    at Object.scanForImportRules (/home/andreas/work/jsdom/lib/jsdom/level2/style.js:119:45)
    at Object.evaluateStylesheet (/home/andreas/work/jsdom/lib/jsdom/level2/style.js:101:22)
    at Object.<anonymous> (/home/andreas/work/jsdom/lib/jsdom/level2/style.js:229:24)
    at Function.dispatch (/home/andreas/work/jsdom/lib/jsdom/level2/events.js:195:42)
    at Object.<anonymous> (/home/andreas/work/jsdom/lib/jsdom/level2/events.js:285:33)
    at Object.dispatchEvent (/home/andreas/work/jsdom/lib/jsdom/level2/html.js:474:55)
    at /home/andreas/work/jsdom/lib/jsdom/level2/events.js:339:14
    at visit (/home/andreas/work/jsdom/lib/jsdom/level1/core.js:36:9)
    at Object.visitTree (/home/andreas/work/jsdom/lib/jsdom/level1/core.js:48:5)
    at Object.<anonymous> (/home/andreas/work/jsdom/lib/jsdom/level2/events.js:337:12)

... but I haven't narrowed it down to a simple failing test because the fix seemed so obvious.

Best regards,
Papandreou

@papandreou

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2011

The code causing the ReferenceError was checked in way back in March (fe9576b), but I only started seeing it recently. git bisect points at ab9398a where a bunch of error reporting was added, so the error probably happened all along, but was silenced before that commit.

@tmpvar

This comment has been minimized.

Copy link
Collaborator

commented Oct 31, 2011

I'll take it, but can you work on a test for this? Or atleast provide some markup/css that causes this explosion?

@tmpvar tmpvar merged commit 3fdd25d into jsdom:master Oct 31, 2011

@papandreou

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2011

Thanks for merging. Wouldn't a test be overkill for a ReferenceError? It was basically a mistyped variable name.

I'm also hesitating because it would probably take at least an hour to boil my huge web app down to a test case.

@tmpvar

This comment has been minimized.

Copy link
Collaborator

commented Oct 31, 2011

Thanks for merging. Wouldn't a test be overkill for a ReferenceError? It was basically a mistyped variable name.

I hear ya, which is why I pulled this in without it.

I'm also hesitating because it would probably take at least an hour to boil my huge web app down to a test case.

Agreed, you should not have to boil your entire webapp down into a single test case. However, it would be nice to know in the future that a specific piece of markup/css does not cause similar failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.