Add support for insertAdjacentElement

• Component: DOM
• Status: RESOLVED FIXED

[Geoffrey Garen]

http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/insertadjacentelement.asp

[Geoffrey Garen]

Created attachment 5638 [details]
Darin's rough cut patch

"A rough first cut of the method only, not the JavaScript binding."

[Anders Carlsson]

I did some investigation on how insertAdjacentElement works in WinIE:

* This function only exists on elements, so it should be moved to ElementImpl
* Passing a non-element object as newChild throws an E_INVALIDARG COM exception
* If the element has no parent and where is "beforeBegin" or "afterEnd", a document fragment is created 
and the elements appended in the correct order. This document fragment isn't returned anywhere
* Passing a bogus string as where also throws an E_INVALIDARG exception.

[Alexander Kellett]

does this need to be bound to HTMLElement or DOMElement?

[Alexander Kellett]

Created attachment 6960 [details]
simple testcase

[Alexander Kellett]

Created attachment 6961 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet

[Alexander Kellett]

andersca, not sure i understand your 3rd point? there is a side effect? does dom tree walking after insertion result in the added element having been inserted into the fragment?

what would be the equiv of a "E_INVALIDARG COM exception" for webkit?

can't really go any further at this point as i don't have ie.
going to mark for review and hope for some feedback.

[Darin Adler]

Comment on attachment 6961 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet

This should be done with an new style IDL-file-based binding, not an old style hand-written binding, because it has no special requirements. So it should be added to Element.idl and kjs_dom.cpp and kjs_dom.h should be left untouched.

[Alexander Kellett]

andersca had already said not to do so.. tho i forget the reasoning.

[Darin Adler]

(In reply to comment #8)
> andersca had already said not to do so.. tho i forget the reasoning.

Anders, why?

[Alexander Kellett]

spoke to anders since my last comment and realized that his comment was in response to something else :P

this works just fine -

+        // IE only extension
+        Node insertAdjacentElement(in DOMString position, 
+                                   in Node newAttr)
+            raises(DOMException);
+

do i need to fill out the exception throwing before committing (after another review)
or is this enough for the moment?

[Darin Adler]

I'd like to see the test cases and exceptions in before we land this. Shouldn't be too difficult to research how it works and make a nice thorough case. I don't want to have to revisit this later.

[Alexander Kellett]

still not sure what andersca's reference to "E_INVALIDARG" implies,
is this just a case of adding that to the DOMException class?

[Anders Carlsson]

Since the JS wrappers in WinIE wrap COM objects, there are some native COM exceptions that are being used in WinIE, for example the E_INVALIDARG exception.

[Alexander Kellett]

Created attachment 7175 [details]
latest testcase

[Alexander Kellett]

Created attachment 7176 [details]
latest testcase

[Alexander Kellett]

Created attachment 7320 [details]
testcases

[Alexander Kellett]

Created attachment 7321 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet

[Darin Adler]

Comment on attachment 7321 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet

Looks fine. r=me

[Eric Seidel (no email)]

Lypie commited this.