WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
51005
Range::extractContents needs more tests
https://bugs.webkit.org/show_bug.cgi?id=51005
Summary
Range::extractContents needs more tests
Ryosuke Niwa
Reported
2010-12-13 21:08:40 PST
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.
Attachments
Patch
(8.58 KB, patch)
2010-12-14 12:46 PST
,
Emil A Eklund
no flags
Details
Formatted Diff
Diff
Patch
(9.35 KB, patch)
2010-12-14 13:35 PST
,
Emil A Eklund
no flags
Details
Formatted Diff
Diff
adds a test
(10.04 KB, patch)
2010-12-14 15:24 PST
,
Ryosuke Niwa
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Emil A Eklund
Comment 1
2010-12-14 12:46:04 PST
Created
attachment 76559
[details]
Patch
Darin Adler
Comment 2
2010-12-14 13:02:01 PST
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
Comment 3
2010-12-14 13:05:36 PST
That's a good idea, I'll see how other tests deal with it and rework it. Thanks.
Emil A Eklund
Comment 4
2010-12-14 13:35:01 PST
Created
attachment 76565
[details]
Patch Changed test to compare outerHTML instead of textContent.
Ryosuke Niwa
Comment 5
2010-12-14 14:00:18 PST
(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 6
2010-12-14 15:21:07 PST
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
Comment 7
2010-12-14 15:24:16 PST
Created
attachment 76580
[details]
adds a test
Ryosuke Niwa
Comment 8
2010-12-14 19:03:37 PST
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
Comment 9
2010-12-14 19:06:37 PST
Committed
r74094
: <
http://trac.webkit.org/changeset/74094
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug