1100662 - Fix browser_notification_tab_switching.js to work in e10s mode

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[:Gijs (he/him)]

This test requires focus to work and does a bunch of direct access to the content window which fails:

765 INFO Console message: [JavaScript Error: "[object CPOW [Exception... "Failure"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/RemoteAddonsChild.jsm :: EventTargetChild.prototype.handleEvent :: line 389"  data: no]]" {file: "resource://gre/modules/RemoteAddonsParent.jsm" line: 488}]

and causes the test to time out.

Comment 1

[Chris Peterson [:cpeterson]]

e10s test bugs should block tracking-e10s=+

Comment 2

[Neil Deakin]

This test has some custom waitForFocus type function, unclear if it is needed or if it can just be changed to use waitForFocus.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I removed the portion of the test that opened a new window and the associating interaction with file_dom_notifications.html that changed focus to the new window. The test still checks that event.defaultPrevented is true as well as that the tabs are not switched, which should be sufficient.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v1.1

The head.js changes weren't needed.

Comment 5

[:Gijs (he/him)]

Comment on attachment 8685134 [details] [diff] [review]
Patch v1.1

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

LGTM

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d1cc0dd57fbe

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for breaking the test: https://treeherder.mozilla.org/logviewer.html#?job_id=5676917&repo=fx-team

https://hg.mozilla.org/integration/fx-team/rev/628b7b9e2c69

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ccb6fcc11d2

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

Although the try run looks pretty noisy, there are no failures for leaving the window around anymore with this tweaked patch.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/0403cb97fb51eb7a05ae2f8091fe031d79ae6ae1
Bug 1100662 - Fix browser_notification_tab_switching.js to work in e10s mode. r=MattN

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0403cb97fb51

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8691423 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: more test coverage for new feature work around push notifications releasing in 44
[User impact if declined]: less test coverage
[Describe test coverage new/current, TreeHerder]: this is updating an automated test
[Risks and why]: none
[String/UUID change made/needed]: none

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8691423 [details] [diff] [review]
Patch v2

test only change. Aurora44+

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cfcf9ab4f705

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/cfcf9ab4f705