Closed Bug 1254557 Opened 9 years ago Closed 9 years ago

Make test_428653.xul work in e10s

Categories

(Toolkit :: View Source, defect, P1)

Product:
Toolkit
Component:
View Source
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla48
Iteration:
48.1 - Mar 21
Tracking Flags:
Tracking Status
e10s + ---
firefox48 --- fixed

People

(Reporter: past, Assigned: past)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxprivacy])

Attachments

(1 file)

MozReview Request: Bug 1254557 - Convert test_428653.xul to an HTML-based chrome mochitest. r?bholley
58 bytes, text/x-review-board-request
bholley
: review+
Details
This is a chrome mochitest that needs to be converted to either a browser chrome test or a plain mochitest.
Blocks: 1216897
Status: NEW → ASSIGNED
Iteration: --- → 48.1 - Mar 21
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [fxprivacy]
Flags: qe-verify? → qe-verify-
Originally I had convinced myself that this test needs to run in e10s, but now that I started working on it I no longer see the point. Bobby, what do you think?
Flags: needinfo?(bobbyholley)
This test doesn't really need to be using a <browser> element, or to anything related to view-source AFAICT. We just need to have some chrome code interacting with a content document. I'm worried that in its current configuration, this test will just cease to work at some point. So I think it should either be converted into a proper mochitest-chrome (html document with a content iframe), or a mochitest-plain that does the chrome stuff via SpecialPowers.Cu.Sandbox.
Flags: needinfo?(bobbyholley)
Attachment #8731167 - Flags: review?(bobbyholley)
Is this what you had in mind? The test remains a mochitest-chrome test, which means it won't run in e10s, but it now uses an HTML document with an iframe.
For some reason MozReview lost the fact that the test file was just renamed and modified only in the preamble and epilogue. The JS code remains the same.
Attachment #8731167 - Flags: review?(bobbyholley)
Comment on attachment 8731167 [details] MozReview Request: Bug 1254557 - Convert test_428653.xul to an HTML-based chrome mochitest. r?bholley https://reviewboard.mozilla.org/r/40405/#review36993 r=me with that one change. ::: toolkit/components/viewsource/test/test_bug428653.html:13 (Diff revision 1) > + <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script> > + <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/> > +</head> > +<body> > + > + <iframe id="content"> </iframe> I think we should probably load a real document here, a la: http://mxr.mozilla.org/mozilla-central/source/dom/base/test/file_empty.html?force=1 Since some of the relevant code here might do different things with about:blank documents.
Attachment #8731167 - Flags: review+
The test fails if I use <iframe id="content" src="file_empty.html"></iframe>: 4 INFO TEST-UNEXPECTED-FAIL | toolkit/components/viewsource/test/test_bug428653.html | Shouldn't have opened document - got true, expected false SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:267:5 testDocumentOpen@chrome://mochitests/content/chrome/toolkit/components/viewsource/test/test_bug428653.html:36:5 The current version of the test is indeed using <browser src="about:blank" />, so perhaps this is to be expected?
Flags: needinfo?(bobbyholley)
(In reply to Panos Astithas [:past] from comment #7) > The test fails if I use <iframe id="content" src="file_empty.html"></iframe>: Is that actually what your code looks like? Because a chrome:// test is loaded from the chrome:// protocol, which means that relative loads will also be from the chrome:// protocol. You need to load the thing relative to http://www.example.com or somesuch to make it load as content. See the examples here: http://mxr.mozilla.org/mozilla-central/search?string=file_empty&find=xpconnect%2Ftests%2Fchrome&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Flags: needinfo?(bobbyholley)
Oops, good point, fixed.
https://hg.mozilla.org/mozilla-central/rev/91966a94be48
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: