User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.130 Safari/537.36 Steps to reproduce: ``` var shadowRoot = document.createElement('a').createShadowRoot(); // Should throw NotSupportedError, however, instead it returns a DocumentFragment (the ShadowRoot) var x = document.importNode(shadowRoot); ``` Actual results: Calling importNode on a document returns a DocumentFragment. Expected results: A NotSupportedError should've been thrown. https://dom.spec.whatwg.org/#dom-document-importnode
Component: Untriaged → DOM
Product: Firefox → Core
The patch there will cause a DataCloneError, the spec here says it should be a NotSupportedError when using `document.importNode`. Although, whether the spec should be a DataCloneError or a NotSupportedError is questionable. I'm in favour of 'NotSupportedError'.
Too many coding styles.. tabs -> spaces.
Comment on attachment 8626892 [details] [diff] [review] bug-1177914.patch Review of attachment 8626892 [details] [diff] [review]: ----------------------------------------------------------------- While this does throw a NotSupportedError, it also outputs a warning that was probably meant to catch unexpected node types. For the shadow root case, it's not unexpected because it's explicitly mentioned by the spec. We should keep the default case of the switch statement and just return NS_ERROR_DOM_NOT_SUPPORTED_ERR from the check for the shadow root. As a side note, it looks like importing a document nodes warns as well, nsIDOMNode::DOCUMENT_NODE should probably have a case in the switch statement that just returns the error without the warning (similar to what we do in nsIDocument::AdoptNode), we could probably do that here too and just add "shadow root or document" to the patch description.
Attachment #8626892 - Flags: review?(wchen) → review-
Comment on attachment 8632866 [details] [diff] [review] Bug 1177914 - throw a NotSupportedError when running document.importNode on a ShadowRoot or Document I'm unsure about the order of the 'case' statements. I've kept the statements that require branching at the top. Sorry it took so long :), I've been pretty busy.
Attachment #8632866 - Flags: review?(wchen)
Assignee: nobody → sam.e.giles
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.