Closed Bug 1265436 Opened 4 years ago Closed 4 years ago

Make dom/tests/browser/browser_bug396843.js work in e10s

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox48 --- fixed

People

(Reporter: tracy, Assigned: tracy)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Modernize browser_bug396842.js

(also, ride along re-enable browser_bug1238427.js, as it wfm locally)

try run just started: https://treeherder.mozilla.org/#/jobs?repo=try&revision=173eded17fe7aa9d6958f75e20cf41baaefc13ef
Attached patch bug_1265436.patch (obsolete) — Splinter Review
I had a couple of incorrect try codes that just resulted in builds but no test runs. Got it correct here:https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa0cb164fa97a303b2fd03dba99ca74a95224d4c

unrelated orange and Mac 10.6 all around busted. The attached patch passes on try.
Attachment #8743907 - Flags: review?(jmathies)
Comment on attachment 8743907 [details] [diff] [review]
bug_1265436.patch

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

::: dom/tests/browser/browser_bug396843.js
@@ +269,2 @@
>  
> +    function newTabTest(location) {

nit - function and its content shouldn't be indented here since it's defined in the global scope now.
Attachment #8743907 - Flags: review?(jmathies) → review+
Attached patch bug_1265436.patch (obsolete) — Splinter Review
Actually the modernization and splitting up the test case into three parts required the entire test case to need white space cleanup.  attached patch addresses that.
Assignee: nobody → twalker
Attachment #8743907 - Attachment is obsolete: true
Attachment #8743918 - Flags: review?(jmathies)
Comment on attachment 8743918 [details] [diff] [review]
bug_1265436.patch

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

::: dom/tests/browser/browser_bug396843.js
@@ +271,5 @@
> +    let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, location);
> +    let doc = content.document;
> +    testInDocument(doc, location);
> +    yield BrowserTestUtils.removeTab(tab);
> +    }

nit - indentation error here for '}'
Attachment #8743918 - Flags: review?(jmathies) → review+
oy, I am still terrible with catching white space, fixed the last in this patch.
Attachment #8743918 - Attachment is obsolete: true
Attachment #8744048 - Flags: review?(jmathies)
Attachment #8744048 - Flags: review?(jmathies) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1c638e1c121c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.