Range::extractContents needs more tests

• Component: DOM
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Currently, editing/execCommand/4920742-2.html is the only layout test that calls extractContents (as far as I checked on Finder).  However, the feature is far from trivial and requires much more rigorous testing.

[Emil A Eklund]

Created attachment 76559 [details]
Patch

[Darin Adler]

Comment on attachment 76559 [details]
Patch

It’s great to have more coverage.

However, textContent is an imprecise way to log the extracted fragment. These tests would be stronger if they used something that shows the details of the DOM nodes of the created fragment rather than just the concatenated text.

I think we have something for dumping markup that Ojan worked on a while back. Or even outerHTML might be better?

[Emil A Eklund]

That's a good idea, I'll see how other tests deal with it and rework it. Thanks.

[Emil A Eklund]

Created attachment 76565 [details]
Patch

Changed test to compare outerHTML instead of textContent.

[Ryosuke Niwa]

(In reply to comment #4)
> Created an attachment (id=76565) [details]
> Patch
> 
> Changed test to compare outerHTML instead of textContent.

We should just use dump-as-markup.js here.

[Eric Seidel (no email)]

Comment on attachment 76559 [details]
Patch

Cleared Darin Adler's review+ from obsolete attachment 76559 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Ryosuke Niwa]

Created attachment 76580 [details]
adds a test

[Ryosuke Niwa]

Thanks for the review, Darin.

(In reply to comment #7)
> Created an attachment (id=76580) [details]
> adds a test

Per discussion with Ojan, I'm adding a comment to dump-as-markup.js as in:
     // FIXME: Have this respect layoutTestController.dumpChildFramesAsText?
     // FIXME: Should we care about framesets?
-    var iframes = node.getElementsByTagName('iframe');
-    for (var i = 0; i < iframes.length; i++) {
-        markup += '\n\nFRAME ' + i + ':\n'
-        try {
-            markup += Markup.get(iframes[i].contentDocument.body.parentElement);
-        } catch (e) {
-            markup += 'FIXME: Add method to layout test controller to get access to cross-origin frames.';
+    // DocumentFragment doesn't have a getElementsByTagName method.
+    if (node.getElementsByTagName) {
+        var iframes = node.getElementsByTagName('iframe');
+        for (var i = 0; i < iframes.length; i++) {
+            markup += '\n\nFRAME ' + i + ':\n'
+            try {
+                markup += Markup.get(iframes[i].contentDocument.body.parentElement);
+            } catch (e) {
+                markup += 'FIXME: Add method to layout test controller to get access to cross-origin frames.';
+            }

[Ryosuke Niwa]

Committed r74094: <http://trac.webkit.org/changeset/74094>