| Summary: | Add test to verify type enforcement in DOM setters | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Pam Greene (IRC:pamg) <pam> | ||||
| Component: | DOM | Assignee: | Adam Barth <abarth> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Pam Greene (IRC:pamg)
2008-12-02 17:25:29 PST
Created attachment 25695 [details]
New test + result
This test could be made much more comprehensive, but to be honest, I don't know enough about underlying DOM structure to know what ought to be possible and what not. These are the only two examples we actually encountered in the wild.
(In reply to comment #1) > This test could be made much more comprehensive, but to be honest, I don't know > enough about underlying DOM structure to know what ought to be possible and > what not. Our usual approach is to make a more comprehensive test without worrying unduly about whether the current behavior is correct or not -- a regression test has lots of value even if it has to be revised later once we understand what's correct and what's not. So my approach if I was writing a test would be to make a long list of functions and make a test that covers them as completely as possible. I'll review this soon; although someone else is welcome to. Still wading through a lot of correspondence after my return from "vacation". Comment on attachment 25695 [details]
New test + result
For a test like this, the preferred form is to have the entire test in a .js file and let the make-js-test-wrappers test create a boilerplate wrapper.
It's a little strange to have this test only these two specific setters. It would be way better to have something that made an attempt to be exhaustive.
r=me
Will land. Sorry for leaving this on the queue. I was hoping to write a more complete test, but I've since gotten mired in other things and won't be getting back to this for a bit. So you're welcome to land it; I'll improve it later. LayoutTests/ChangeLog: locally modified LayoutTests/fast/dom/null-document-xmlhttprequest-open.html: locally modified LayoutTests/fast/dom/xmlhttprequest-invalid-values.html: locally modified M LayoutTests/ChangeLog A fast/dom/setter-type-enforcement-expected.txt A fast/dom/setter-type-enforcement.html Committed r39363 A fast/dom/setter-type-enforcement-expected.txt A fast/dom/setter-type-enforcement.html M LayoutTests/ChangeLog r39363 = 6e34f4ececac5729488eae7cb0d6359322c15107 (trunk) No worries. It makes me feel useful to clean out some things in the commit queue. > A fast/dom/setter-type-enforcement-expected.txt > A fast/dom/setter-type-enforcement.html > M LayoutTests/ChangeLog Moved the tests from root level into LayoutTests/fast/dom/ in r39375. Test fails on buildbot. I thought it passed on my machine, but I might have been mistaken. Here's the diff: --- layout-test-results/fast/dom/setter-type-enforcement-expected.txt 2008-12-18 18:04:24.000000000 -0800 +++ layout-test-results/fast/dom/setter-type-enforcement-actual.txt 2008-12-18 18:04:24.000000000 -0800 @@ -8,12 +8,6 @@ PASS successfullyParsed is true TEST COMPLETE - -Tests type enforcement on DOM setters. - -On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". - - PASS document.body = nodelist; threw exception Error: HIERARCHY_REQUEST_ERR: DOM Exception 3. PASS table.tHead = nodelist; threw exception Error: NOT_FOUND_ERR: DOM Exception 8. PASS successfullyParsed is true (In reply to comment #9) > Test fails on buildbot. I thought it passed on my machine, but I might have > been mistaken. Patching error. Both the test and the expected results in the repository were doubled copies of the patch attached here. See http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/setter-type-enforcement.html?rev=39375 http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/setter-type-enforcement-expected.txt?rev=39375 I'll resubmit corrected copies. Resubmitted in r39410. (In reply to comment #10) > Patching error. Both the test and the expected results in the repository were > doubled copies of the patch attached here. See Oops. Sorry about that. I must have screwed up my pre-submit git rebase somehow. |