Make nsFocusManager::Blur() hand off focus one level up only

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P2
normal
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

(Blocks 1 bug)

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Fission Milestone:M2, firefox67 fixed)

Details

(Whiteboard: [fission-event-m1])

Attachments

(1 attachment)

Assignee

Description

4 months ago

When focus traversal moves out of an out-of-process iframe, in nsFocusManager::Blur, change TabChild::SendDispatchFocusToTopLevelWindow() such that its semantics are to dispatch focus up one level instead of dispatching to the top level. In a content process that’s associated with a top-level browsing context, this should work as before. In out-of-process iframes, there is a RemoteFrameParent associated with the TabParent, so this should bounce via the RemoteFrameParent to the associated RemoteFrameChild, which should move the focus out of the iframe into the next focusable in the browsing context that is the browsing context parent of the out-of-process iframe.

Priority: -- → P2
Whiteboard: [fission-event-m1]

Updated

3 months ago
Fission Milestone: --- → M2
Assignee

Comment 1

3 months ago

A RemoteFrameParent holds a strong ref to the corresponding TabParent. What's the proper way to traverse the connection the other way: from within TabParent finding the corresponding RemoteFrameParent if one exists?

Flags: needinfo?(rhunt)

Comment 2

3 months ago

There's currently not a way to do this. The TabParent should hold an optional reference to its parent 'RemoteFrameParent' if it exists. (note, these names will change in bug 1523072)

Flags: needinfo?(rhunt)
Assignee

Comment 3

3 months ago

(In reply to Ryan Hunt [:rhunt] from comment #2)

There's currently not a way to do this. The TabParent should hold an optional reference to its parent 'RemoteFrameParent' if it exists. (note, these names will change in bug 1523072)

Thanks. I'll add a non-owning pointer in the other direction and will make RemoteFrameParent null it out on its destruction.

New finding:
While focus movement down is async for IPC purposes, focus movement up is sync. However, the IPDL compiler says: "sync parent-to-child messages are verboten", so I can't make the RemoteFrameParent to RemoteFrameChild bounce sync. Not sure what the implications of this are yet.

Assignee

Comment 4

3 months ago

jimm, what's the reason for making cross-process focus traversal upwards across process boundaries a synchronous message in https://hg.mozilla.org/mozilla-central/rev/4ba02f888904 ? That is, what problems should I expect from making bounce from RemoteFrameParent to RemoteFrameChild asynchronous in the case where focus isn't moving from content to chrome but moving from content to content across processes?

Flags: needinfo?(jmathies)
Assignee

Comment 6

2 months ago

Looks like TabChild::SendDispatchFocusToTopLevelWindow() is not the right thing and TabChild::SendMoveFocus() is. It's unclear to me why TabChild::SendDispatchFocusToTopLevelWindow() exists also.

Assignee

Comment 7

2 months ago

Moving needinfo to bug 1534586.

Flags: needinfo?(jmathies)
Assignee

Updated

2 months ago
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED

Comment 8

2 months ago
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e165cd680b9
Make nsFocusManager::Blur() hand off focus one level up only. r=NeilDeakin

Comment 9

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.