601277 - Write tests for document.domain

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Tests

Comment 2

[Mounir Lamouri (:mounir)]

FWIW, these tests are no longer working.

Comment 3

[Johnny Stenback (:jst)]

We'll need this test when mrbkap works on further document.domain related changes for beta8, so marking a blocker.

Comment 4

[Johnny Stenback (:jst)]

We don't need to worry about this for beta8, pushing to beta9.

Comment 5

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)

Comment 6

[Mounir Lamouri (:mounir)]

Re-setting the CC list assuming this change wasn't intentional.

Comment 7

[LegNeato]

Fixing fields my automated script accidentally blanked. Apologies for the bugspam

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

I'm keeping this as a blocker because I believe in test driven development yatta yatta, but tests don't need permission for approval so I question whether or not this even needs soft blocking status.

It needs to be done, absolutely, but does it need to keep us from shipping? I assert not!

Comment 9

[Mounir Lamouri (:mounir)]

Attachment: Tests

Updated tests to make sure the test actually finish on trunk (currently 8 passes and 10 fails).

Comment 10

[Mounir Lamouri (:mounir)]

(In reply to comment #8)
> I'm keeping this as a blocker because I believe in test driven development
> yatta yatta, but tests don't need permission for approval so I question whether
> or not this even needs soft blocking status.

FWIW, the real purpose of the bug is to make trunk pass this test.

Comment 11

[(no longer active)]

We're past feature freeze.  This sounds like a non-blocker to me.

Comment 12

[Johnny Stenback (:jst)]

Yeah, we should get these tests in in a form where they pass, but we did change our minds on what the exact security policy is wrt document.domain. Either way, this is not a blocker any more due to the policy change.

Comment 13

[Bobby Holley (:bholley)]

Attachment: Better tests. v1

I spent some time with the tests on the bug this morning and found them inscrutable and impossible to extend. Here are some much cleaner ones.

Comment 14

[Bobby Holley (:bholley)]

Note that we're now explicitly testing for the behavior I proposed here: http://lists.w3.org/Archives/Public/public-script-coord/2012AprJun/0120.html

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 637462 [details] [diff] [review]
Better tests. v1

Review of attachment 637462 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/tests/chrome/test_documentdomain.xul
@@ +32,5 @@
> +  function startTest() {
> +
> +    // Grab all the content windows. We waive Xray here, and add it back with
> +    // XPCNativeWrapper whenever we pass references into content.
> +    var win1A = document.getElementById('test1A').contentWindow.wrappedJSObject;

All of this futzing around with .wrappedJSObject and XPCNativeWrapper shouldn't matter for the case where we're passing object around... I think you should be able to nuke it.

Comment 16

[Bobby Holley (:bholley)]

Pushed to m-i:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c9b038fa6956

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c9b038fa6956