1234828 - Re-enable some e10s tests in toolkit/content/tests/browser

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Comment 1

[Neil Deakin]

Attachment: browser_f7_caret_browsing.js

Comment 2

[Neil Deakin]

Attachment: browser_quickfind_editable.js

Comment 3

[Neil Deakin]

Attachment: browser_bug594509.js

Comment 4

[Neil Deakin]

Attachment: browser_Finder.js

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8701462 [details] [diff] [review]
browser_quickfind_editable.js

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

Hihi, nice how this work pays off!

Comment 6

[Mike de Boer [:mikedeboer]]

Comment on attachment 8701464 [details] [diff] [review]
browser_Finder.js

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

I like it? I'd advise to do a try run before pushing?

::: toolkit/modules/tests/browser/browser_Finder.js
@@ +1,3 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

While you're here, perhaps it's good to change this to the CC-license that we generally use for test code?

@@ +3,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  var Ci = Components.interfaces;
>  
> +add_task(function *()

nit: please format as `add_task(function* () {`, which I think matches browser/ style closer.

@@ +43,2 @@
>  
> +  yield ContentTask.spawn(tab.linkedBrowser, { }, function* (arg) {

nit: I think empty object literals don't need additional spacing.

@@ +53,2 @@
>  
> +  yield ContentTask.spawn(tab.linkedBrowser, { }, function* (arg) {

nit: same.

Comment 7

[Mike de Boer [:mikedeboer]]

(In reply to Mike de Boer [:mikedeboer] from comment #6)
> I like it? I'd advise to do a try run before pushing?

This meant to say: 'I like it!!'

Comment 8

[Neil Deakin]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2bd0398b854&selectedJob=15017718

Comment 9

[:Gijs (he/him)]

Comment on attachment 8701463 [details] [diff] [review]
browser_bug594509.js

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

r+ assuming green try w/ retriggers.

::: toolkit/content/tests/browser/browser_bug594509.js
@@ +8,1 @@
>    gBrowser.removeTab(tab);

nit:

yield BrowserTestUtils.removeTab(tab);

Comment 10

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfaafbd4bcf9bf7f26a052873984896d5ec50570
Bug 1234828, re-enable browser_quickfind_editable.js in e10s, r=mikedeboer

https://hg.mozilla.org/integration/mozilla-inbound/rev/f87f710e2f1022a6300d2da46d8cfc34ebea9706
Bug 1234828, re-enable browser_bug594509.js in e10s, r=gijs

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2fdec91ce98b76ffc4e9e12274f2f2c83738e50
Bug 1234828, re-enable browser_Finder.js in e10s, r=mikedeboer

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cfaafbd4bcf9
https://hg.mozilla.org/mozilla-central/rev/f87f710e2f10
https://hg.mozilla.org/mozilla-central/rev/c2fdec91ce98