Bug 22612 – Add test to verify type enforcement in DOM setters

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Pam Greene (IRC:pamg)]

Another test.

[Pam Greene (IRC:pamg)]

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.

[Darin Adler]

(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".

[Darin Adler]

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

[Adam Barth]

Will land.

[Pam Greene (IRC:pamg)]

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.

[Adam Barth]

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)

[Adam Barth]

No worries.  It makes me feel useful to clean out some things in the commit queue.

[Pam Greene (IRC:pamg)]

>         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.

[Adam Barth]

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

[Pam Greene (IRC:pamg)]

(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.

[Pam Greene (IRC:pamg)]

Resubmitted in r39410.

[Adam Barth]

(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.