Bug 6520 - Add support for insertAdjacentElement
Summary: Add support for insertAdjacentElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-12 20:39 PST by Geoffrey Garen
Modified: 2006-03-27 17:07 PST (History)
3 users (show)

See Also:


Attachments
Darin's rough cut patch (2.19 KB, patch)
2006-01-12 20:40 PST, Geoffrey Garen
no flags Details | Formatted Diff | Diff
simple testcase (770 bytes, text/html)
2006-03-09 11:23 PST, Alexander Kellett
no flags Details
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet (3.70 KB, patch)
2006-03-09 11:37 PST, Alexander Kellett
darin: review-
Details | Formatted Diff | Diff
latest testcase (1.59 KB, text/html)
2006-03-19 09:57 PST, Alexander Kellett
no flags Details
latest testcase (1.87 KB, text/html)
2006-03-19 10:51 PST, Alexander Kellett
no flags Details
testcases (2.66 KB, patch)
2006-03-26 21:06 PST, Alexander Kellett
darin: review+
Details | Formatted Diff | Diff
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet (4.13 KB, patch)
2006-03-26 21:06 PST, Alexander Kellett
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Geoffrey Garen 2006-01-12 20:40:26 PST
Created attachment 5638 [details]
Darin's rough cut patch

"A rough first cut of the method only, not the JavaScript binding."
Comment 2 Anders Carlsson 2006-01-13 08:32:57 PST
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.

Comment 3 Alexander Kellett 2006-03-08 14:08:35 PST
does this need to be bound to HTMLElement or DOMElement?
Comment 4 Alexander Kellett 2006-03-09 11:23:01 PST
Created attachment 6960 [details]
simple testcase
Comment 5 Alexander Kellett 2006-03-09 11:37:06 PST
Created attachment 6961 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet
Comment 6 Alexander Kellett 2006-03-11 13:41:57 PST
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.
Comment 7 Darin Adler 2006-03-11 22:10:10 PST
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.
Comment 8 Alexander Kellett 2006-03-12 02:21:58 PST
andersca had already said not to do so.. tho i forget the reasoning.
Comment 9 Darin Adler 2006-03-13 20:49:28 PST
(In reply to comment #8)
> andersca had already said not to do so.. tho i forget the reasoning.

Anders, why?
Comment 10 Alexander Kellett 2006-03-13 21:50:24 PST
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?
Comment 11 Darin Adler 2006-03-14 10:56:02 PST
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.
Comment 12 Alexander Kellett 2006-03-14 12:20:46 PST
still not sure what andersca's reference to "E_INVALIDARG" implies,
is this just a case of adding that to the DOMException class?
Comment 13 Anders Carlsson 2006-03-14 12:55:32 PST
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.
Comment 14 Alexander Kellett 2006-03-19 09:57:40 PST
Created attachment 7175 [details]
latest testcase
Comment 15 Alexander Kellett 2006-03-19 10:51:11 PST
Created attachment 7176 [details]
latest testcase
Comment 16 Alexander Kellett 2006-03-26 21:06:18 PST
Created attachment 7320 [details]
testcases
Comment 17 Alexander Kellett 2006-03-26 21:06:54 PST
Created attachment 7321 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet
Comment 18 Darin Adler 2006-03-26 22:46:45 PST
Comment on attachment 7321 [details]
patch taking into account move to Element, and implementing bindings, no exceptions thrown yet

Looks fine. r=me
Comment 19 Eric Seidel (no email) 2006-03-27 17:07:02 PST
Lypie commited this.