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.

Sign up for GitHub

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

Jump to bottom

Make testsuite work on IE9 #21

Closed
cure53 opened this issue Apr 11, 2014 · 8 comments
Closed

Make testsuite work on IE9 #21

cure53 opened this issue Apr 11, 2014 · 8 comments
Labels
bug enhancement

Comments

@cure53
Copy link
Owner

cure53 commented Apr 11, 2014

So far the test-suite does not work at all on IE9, dying with an error stating that innerHTML of undefined cannot be set. QUnit issue? (@fhemberger)
@fhemberger
Copy link
Contributor

Nope, it's this statement: https://github.com/cure53/DOMPurify/blob/master/purify.js#L178

/* Cover IE9's buggy outerHTML behavior */
if(dom.body === null) {
    dom.body.innerHTML = dirty;
}

I don't know what your original intention was … ;)

Second issue: https://github.com/cure53/DOMPurify/blob/master/test/index.html#L29

QUnit.assert.contains = function( needle, haystack, message ) {
    var actual = haystack.indexOf(needle) > -1;
    QUnit.push(actual, actual, needle, message);
};

This doesn't seem to work when you return early and return an empty sting (https://github.com/cure53/DOMPurify/blob/master/purify.js#L390) (cc @mathiasbynens):

Expected:  ""
Result:  false

@cure53
Copy link
Owner Author

cure53 commented Apr 15, 2014

In IE9, outerHTML and innerHTML in virtual documents is broken. The seemingly hackish code covers that and enables DOMPurify to work fine nevertheless. Unfortunately the tests disagree. Workarounds are welcome :)

@fhemberger
Copy link
Contributor

Well, null shouldn't be an object and have any properties. And it's exactly this line that breaks all the tests in IE9, because of the attempt to write to null.innerHTML.

@codylindley
Copy link

This is breaking for me in ie9 (not the tests).

https://www.dropbox.com/s/mb490tr9xpxu080/Screenshot%202014-04-15%2014.51.43.png

@codylindley
Copy link

I'm not sure the intent, but using innerHTML instead of outer, works. But it might negate your intention.

@cure53
Copy link
Owner Author

cure53 commented Apr 16, 2014

I am currently revising this issue, you guys are right. Additional code is needed for IE9 to work fine.

@cure53
Copy link
Owner Author

cure53 commented Apr 16, 2014

I added a fix for the innerHTML issue shown above, yet are close to believe that based on the flawed outerHTML behavior on MSIE9 we might not be able to fully support this browser.

I ran the tests and they do work now - but many of them yield results that are beyond reason (closing <body> element in the middle of a HTML string, double-open links, absurd XML processing instructions for MathML strings etc.).

So far my conclusion is: DOMPurify works on IE9, produces safe output, but to make all tests go green we'd have to start accepting absurd HTML that might have structural flaws and produce results we cannot observe on any other browser. Thoughts?

@cure53
Copy link
Owner Author

cure53 commented Sep 11, 2014

Closed for inactivity, no bug reports from IE9. Re-open if necessary.

@cure53 cure53 closed this as completed Sep 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@codylindley @fhemberger @cure53