Closed
Bug 1269812
Opened 8 years ago
Closed 8 years ago
enable test_bug383369.html and test_unsecureRedirect.html for e10s
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: keeler, Assigned: keeler)
References
(Blocks 1 open bug)
Details
(Whiteboard: [psm-assigned])
Attachments
(1 file)
test_bug383369.html and test_unsecureRedirect.html aren't running in e10s-mode right now. AFAICT test_unsecureRedirect.html works fine with no changes, but test_bug383369.html needs some slight modifications.
Updated•8 years ago
|
Blocks: e10s-tests
tracking-e10s:
--- → +
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/50203/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50203/
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50203/diff/1-2/
Attachment #8748334 -
Attachment description: MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html → MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r?Cykesiopka
Attachment #8748334 -
Flags: review?(cykesiopka.bmo)
Comment 3•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap https://reviewboard.mozilla.org/r/50203/#review47409 Looks good! (Admittedly I don't know much about e10s-ifying tests, but the tests now work, and the changes look correct to me, so good enough.) ::: security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html:10 (Diff revision 2) > <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script> > <script type="text/javascript" src="mixedContentTest.js"></script> > <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" /> > > <script class="testbody" type="text/javascript"> > + /* global sendAsyncMessage */ Nit: sendAsyncMessage() isn't really global, so please add a comment that this is here just to suppress ESLint no-undef checks.
Attachment #8748334 -
Flags: review?(cykesiopka.bmo) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8748334 -
Flags: review?(mrbkap)
Assignee | ||
Comment 4•8 years ago
|
||
Thanks! r? to mrbkap for the e10s-ifying aspect.
Updated•8 years ago
|
Attachment #8748334 -
Flags: review?(mrbkap) → review+
Comment 5•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap https://reviewboard.mozilla.org/r/50203/#review47643 ::: security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html:70 (Diff revision 2) > - function afterNavigationTest() > + function afterNavigationTest() {} > - { > - } > > - testCleanUp = function cleanup() > - { > + testCleanUp = function cleanup() { > + SpecialPowers.loadChromeScript(function() { I tend to prefer a single script that initializes itself and has a "cleanup" message that you can call sendSyncMessage for, but that's a matter of style, so I leave that up to you.
Assignee | ||
Comment 6•8 years ago
|
||
https://reviewboard.mozilla.org/r/50203/#review47409 > Nit: sendAsyncMessage() isn't really global, so please add a comment that this is here just to suppress ESLint no-undef checks. Thanks - added the comment.
Assignee | ||
Comment 7•8 years ago
|
||
https://reviewboard.mozilla.org/r/50203/#review47643 > I tend to prefer a single script that initializes itself and has a "cleanup" message that you can call sendSyncMessage for, but that's a matter of style, so I leave that up to you. Thanks for the review. I left the test cleanup as it was so there's less to uplift to aurora, but it does look like some refactoring can be done (this test is the only one that actually defines testCleanUp, so it doesn't seem necessary as generic functionality any more).
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50203/diff/2-3/
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50203/diff/3-4/
Attachment #8748334 -
Attachment description: MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r?Cykesiopka → MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c6251c984ad6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap Approval Request Comment [Feature/regressing bug #]: e10s [User impact if declined]: less e10s test coverage [Describe test coverage new/current, TreeHerder]: this is the test [Risks and why]: low - it's just a test [String/UUID change made/needed]: none
Attachment #8748334 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8748334 [details] MozReview Request: bug 1269812 - e10s-ify test_bug383369.html and test_unsecureRedirect.html r=Cykesiopka r=mrbkap Oh, wait, I guess since these changes are test-only, I don't need to wait for approval for this.
Attachment #8748334 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 14•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ac470f026c69f90c82798f15dbf9d538fc99b68e
status-firefox48:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•