Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 601277 - Write tests for document.domain
: Write tests for document.domain
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Mounir Lamouri (:mounir)
: Jet Villegas (:jet)
Depends on: 729150
Blocks: CVE-2012-3985
  Show dependency treegraph
Reported: 2010-10-01 16:06 PDT by Mounir Lamouri (:mounir)
Modified: 2012-07-12 09:35 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Tests (7.79 KB, patch)
2010-10-11 03:36 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Tests (7.95 KB, patch)
2011-01-27 08:36 PST, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Better tests. v1 (6.77 KB, patch)
2012-06-28 05:11 PDT, Bobby Holley (:bholley) (busy with Stylo)
mrbkap: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2010-10-01 16:06:52 PDT

Comment 1 Mounir Lamouri (:mounir) 2010-10-11 03:36:28 PDT
Created attachment 482208 [details] [diff] [review]
Comment 2 Mounir Lamouri (:mounir) 2010-10-28 04:11:59 PDT
FWIW, these tests are no longer working.
Comment 3 Johnny Stenback (:jst, 2010-11-05 10:41:41 PDT
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, 2010-11-12 16:23:02 PST
We don't need to worry about this for beta8, pushing to beta9.
Comment 5 christian 2011-01-04 15:43:10 PST
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) 2011-01-04 15:52:31 PST
Re-setting the CC list assuming this change wasn't intentional.
Comment 7 christian 2011-01-04 18:14:18 PST
Fixing fields my automated script accidentally blanked. Apologies for the bugspam
Comment 8 Mike Beltzner [:beltzner, not reading bugmail] 2011-01-21 10:29:36 PST
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) 2011-01-27 08:36:46 PST
Created attachment 507485 [details] [diff] [review]

Updated tests to make sure the test actually finish on trunk (currently 8 passes and 10 fails).
Comment 10 Mounir Lamouri (:mounir) 2011-01-27 08:48:01 PST
(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 :Ehsan Akhgari (Away Oct 25 - Nov 9) 2011-02-09 21:50:30 PST
We're past feature freeze.  This sounds like a non-blocker to me.
Comment 12 Johnny Stenback (:jst, 2011-02-11 10:20:07 PST
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) (busy with Stylo) 2012-06-28 05:11:57 PDT
Created attachment 637462 [details] [diff] [review]
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) (busy with Stylo) 2012-06-28 05:12:25 PDT
Note that we're now explicitly testing for the behavior I proposed here:
Comment 15 Blake Kaplan (:mrbkap) 2012-07-10 16:30:16 PDT
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) (busy with Stylo) 2012-07-12 01:12:17 PDT
Pushed to m-i:
Comment 17 Ed Morley [:emorley] 2012-07-12 09:35:56 PDT

Note You need to log in before you can comment on or make changes to this bug.