Breakdown: [e10s] re-enable tests disabled due to focus related failures

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: markh, Assigned: Neil Deakin)

Tracking

(Blocks: 2 bugs)

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

Firefox Tracking Flags

(e10sm6+)

Details

(Reporter)

Description

4 years ago
eg, nsIFocusManager.getFocusedElementForWindow() will throw NS_ERROR_ILLEGAL_VALUE when called from chrome and passed a content window.
Blocks: 963359
Blocks: 950073
I don't think this really affects dogfooding, so I'm removing the dependency.
No longer blocks: 963359
tracking-e10s: --- → +
Blocks: 997462

Updated

4 years ago
No longer blocks: 950073
Flags: firefox-backlog+
Summary: [e10s] focusManager doesn't work correctly with e10s → Breakdown: [e10s] focusManager doesn't work correctly with e10s
Whiteboard: p=0 [qa-]
Blocks: 1030318
No longer blocks: 997462
Blocks: 1083269
Flags: qe-verify-

Updated

3 years ago
Whiteboard: p=0 [qa-]

Updated

3 years ago
Points: --- → 5

Updated

3 years ago
Blocks: 1093642
These DevTools bugs should probably block e10s from riding to Aurora.
tracking-e10s: + → m6+

Updated

3 years ago
Blocks: 1100653

Updated

3 years ago
Blocks: 1100662

Updated

3 years ago
Blocks: 1102331
(Assignee)

Updated

3 years ago
Summary: Breakdown: [e10s] focusManager doesn't work correctly with e10s → Breakdown: [e10s] focusManager.getFocusedElementForWindow doesn't work correctly with e10s
(Assignee)

Updated

3 years ago
Blocks: 1125897
It's very unclear what this bug is and what needs to be done here. Neil can you take this and either fix or at least make it clear exactly what FIXED means for this bug?
Flags: needinfo?(enndeakin)
(Assignee)

Comment 4

3 years ago
nsIFocusManager.getFocusedElementForWindow() doesn't work when passed a content window, so any callers of this will fail. This method is used to get the focused child frame within a window. We can't fix existing callers though without making this a synchronous call and waiting. Or existing callers will need to call an asynchronous version that could be implemented.

I'm not sure what the original reason for the filing this bug though; that is if there was a specific caller that was broken. Mark?

I guess the first step is to enumerate all of the callers and identify what changes are needed. Perhaps this should be a breakdown type of bug.
Flags: needinfo?(enndeakin) → needinfo?(mhammond)
(Reporter)

Comment 5

3 years ago
This bug was opened to track the disabling of tests with e10s.  It looks like 8 remain disabled - https://dxr.mozilla.org/mozilla-central/search?q=921935&case=false.  I haven't looked into these too deeply - I just annotated the manifest with this bug number when ever a test looked like some method with "focus" in the name caused the test failure.
Flags: needinfo?(mhammond)
(Assignee)

Comment 6

3 years ago
OK, so some of these tests are just expecting waitForFocus to work. That was fixed by bug 1083269, so these tests can be re-enabled, however there see to be other issues that prevent the tests from working:

Tests:

browser/base/content/test/general/browser_bug749738.js
  - appears to just work as is
browser/base/content/test/general/browser_tabfocus.js
  - this is the main test for focus navigation in tabs. It will need to be adapted.
browser/base/content/test/general/browser_typeAheadFind.js
browser/base/content/test/general/browser_visibleFindSelection.js
 - minor changes needed to fix focus issues but these might rely on some findbar initialization timing

browser/devtools/markupview/test/head.js
  - appears to have been rewritten to not use waitForFocus. This could just be undone.
browser/devtools/storage/test/head.js
  - appears to have been rewritten to not use waitForFocus. This could just be undone.

toolkit/content/tests/browser_bug295977_autoscroll_overflow.js
toolkit/content/tests/browser_keyevents_during_autoscrolling.js
 - child process crashes when running these tests
Summary: Breakdown: [e10s] focusManager.getFocusedElementForWindow doesn't work correctly with e10s → Breakdown: [e10s] re-enable tests disabled due to focus related failures
(Assignee)

Updated

3 years ago
Blocks: 1131562
(Assignee)

Updated

3 years ago
Blocks: 1102015
(Assignee)

Updated

3 years ago
Blocks: 1131575
(Assignee)

Updated

3 years ago
Blocks: 1131579
(Assignee)

Updated

3 years ago
Blocks: 1131626
(Assignee)

Updated

3 years ago
Depends on: 1094252
(Assignee)

Updated

3 years ago
Blocks: 1131656
(Assignee)

Updated

3 years ago
Blocks: 1094252
No longer depends on: 1094252

Updated

3 years ago
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Iteration: --- → 38.3 - 23 Feb
(Assignee)

Comment 7

3 years ago
I added some bugs and commented on each indicating what needs to be changed in order to get the test to work. This list is the focus related bugs that are disabled. Bug 1080801 contains other disabled browser tests.

Bug 1131656 in a non-test bug that will fix or help fix some of these tests.
(Assignee)

Comment 8

3 years ago
As a breakdown bug, this should be complete.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
See Also: → bug 1030318
You need to log in before you can comment on or make changes to this bug.