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

Error in getOuterHTML for non-native DOM implementations #274

Closed
bwrrp opened this issue Jun 19, 2014 · 1 comment · Fixed by #275
Closed

Error in getOuterHTML for non-native DOM implementations #274

bwrrp opened this issue Jun 19, 2014 · 1 comment · Fixed by #275

Comments

@bwrrp
Copy link

bwrrp commented Jun 19, 2014

During testing of our JS-based DOM implementation and any code using it, we frequently run into an error in chai when returned nodes do not match an expected value. In this case, chai attempts to pretty-print the nodes as HTML.

As our library is a minimal XML DOM instead of HTML, it does not support the outerHTML property. The code then attempts to add the JS-based node-like object to a container element created from the global (native) document, which obviously fails:

Error: Failed to execute 'appendChild' on 'Node': The new child element is null.
    at Error (native)
    at getOuterHTML (http://localhost:7357/node_modules/chai/chai.js:4136:15)
    at formatValue (http://localhost:7357/node_modules/chai/chai.js:4178:12)
    at formatProperty (http://localhost:7357/node_modules/chai/chai.js:4339:15)
    at http://localhost:7357/node_modules/chai/chai.js:4257:14
    at Array.map (native)
    at formatValue (http://localhost:7357/node_modules/chai/chai.js:4256:19)
    at formatProperty (http://localhost:7357/node_modules/chai/chai.js:4339:15)
    at formatArray (http://localhost:7357/node_modules/chai/chai.js:4300:19)
    at formatValue (http://localhost:7357/node_modules/chai/chai.js:4254:14)

I'm not sure what the best way would be to fix this. The code should at least use the ownerDocument of the incoming node, to ensure the same document implementation is used. It should probably also be more defensive in either detecting whether a node is an HTML element (as opposed to an XML element, which does not have inner/outerHTML). Not printing a pretty value is preferable to failing tests with an internal Error instead of a failed expectation.

@DrRataplan
Copy link
Contributor

As far as I can tell, outerHTML is supported on HTMLElements on all browsers and PhantomJS.

I'd propose removing the getOuterHTML function, replacing it with testing for existence of node.outerHTML, if that fails, try XMLSerializer#serializeToString, if that fails (by throwing), resume the normal flow, eventually falling back to pretty printing it as an object.

I'll write up a pull request somewhere this afternoon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants