Fix browser_notification_tab_switching.js to work in e10s mode

RESOLVED FIXED in Firefox 44, Firefox OS v2.5

Status

()

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Gijs, Assigned: jaws)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 45
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +
qe-verify -

Firefox Tracking Flags

(e10s+, firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Flags: qe-verify-
Flags: in-testsuite+
Flags: firefox-backlog+
e10s test bugs should block tracking-e10s=+
tracking-e10s: ? → +
This test has some custom waitForFocus type function, unclear if it is needed or if it can just be changed to use waitForFocus.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Created attachment 8685130 [details] [diff] [review]
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.
Attachment #8685130 - Flags: review?(MattN+bmo)
Created attachment 8685134 [details] [diff] [review]
Patch v1.1

The head.js changes weren't needed.
Attachment #8685130 - Attachment is obsolete: true
Attachment #8685130 - Flags: review?(MattN+bmo)
Attachment #8685134 - Flags: review?(MattN+bmo)
Attachment #8685134 - Flags: review?(MattN+bmo) → review?(gijskruitbosch+bugs)
Comment on attachment 8685134 [details] [diff] [review]
Patch v1.1

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

LGTM
Attachment #8685134 - Flags: review?(gijskruitbosch+bugs) → review+
Keywords: checkin-needed
Created attachment 8691423 [details] [diff] [review]
Patch v2

Although the try run looks pretty noisy, there are no failures for leaving the window around anymore with this tweaked patch.
Attachment #8685134 - Attachment is obsolete: true
Flags: needinfo?(jaws)
Attachment #8691423 - Flags: review+
Keywords: checkin-needed
Keywords: checkin-needed

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0403cb97fb51
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
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
Attachment #8691423 - Flags: approval-mozilla-aurora?
Comment on attachment 8691423 [details] [diff] [review]
Patch v2

test only change. Aurora44+
Attachment #8691423 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

3 years ago
status-firefox44: --- → affected

Comment 14

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/cfcf9ab4f705
status-firefox44: affected → fixed
You need to log in before you can comment on or make changes to this bug.