Add LayersID field to WidgetEvent

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P2
normal
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

(Blocks 2 bugs)

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Fission Milestone:M1, firefox67 fixed)

Details

(Whiteboard: [fission-event-m1])

Attachments

(1 attachment)

Assignee

Description

4 months ago

Add a field to WidgetEvent that can hold a LayersID value or a "no LayersID" value.

Add a transformation matrix field to WidgetEvent (or, possibly, a subclass; maybe introduce a new class between WidgetInputEvent and the mouse and touch subclasses thereof).

Ensure that these fields survive remoting between the chrome process and the compositor when the two aren't in the same process.

Assignee

Updated

4 months ago
Blocks: 1524229
Assignee

Updated

4 months ago
Blocks: 1524231

The matrix at least should only be relevant to WidgetInputEvent and subclasses, so it probably belongs in WidgetInputEvent. In general APZ hit testing only works on input events, not WidgetEvent in general.

Assignee

Updated

4 months ago
Blocks: 1524239
Assignee

Updated

4 months ago
Blocks: 1524240

See https://bugzilla.mozilla.org/show_bug.cgi?id=1524239#c1 as the approach outlined there might make this bug unnecessary. I don't have particularly strong feelings about it either way.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)

See https://bugzilla.mozilla.org/show_bug.cgi?id=1524239#c1 as the approach outlined there might make this bug unnecessary. I don't have particularly strong feelings about it either way.

Copying over a relevant response from that bug (it doesn't really belong there as that bug is about keyboard events):

(In reply to Botond Ballo [:botond] from comment #2)

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)

So one question is whether we actually need to store this information in the input event itself. Right now the results of the APZ hit test are returned to the caller via the ScrollableLayerGuid out-param in ReceiveInputEvent code and the guid contains the LayersId.

That might not be the right LayersId, though.

Consider a mouse event over a non-scrollable OOP iframe inside a scrollable parent frame. The guid computed for APZ purposes will reference the nearest enclosing scrollable layer, which in this case is the parent, but presumably we want to dispatch the event to the process of the OOP iframe.

(In reply to Botond Ballo [:botond] from comment #3)

Consider a mouse event over a non-scrollable OOP iframe inside a scrollable parent frame. The guid computed for APZ purposes will reference the nearest enclosing scrollable layer, which in this case is the parent, but presumably we want to dispatch the event to the process of the OOP iframe.

In more concrete terms:

  • what APZ wants: hit-test to get a HitTestingTreeNode, then walk up the tree to find the nearest scrollable node
  • what Fission wants: hit-test to get a HitTestingTreeNode, then walk up the tree to find the nearest process-root node

They share part of the work, but the outcome could be different.

Yeah that's a good point. Very well, carry on :)

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

Updated

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

Updated

3 months ago
Blocks: 1528725
Assignee

Comment 7

3 months ago

Bug 1529237 strongly suggests that the design of having APZ event processing attach a matrix to each event is a bad design, since it looks like TabParent needs to be able to ask APZ "What's the matrix for transforming between the chrome process coordinates and my TabChild's coordinates?" if we need that facility anyway for TabParent::Recv*, we might as well use it for TabParent::Send*.

Assignee

Comment 8

3 months ago

Scoping down the bug per the previous comment.

Summary: Add LayersID and transform matrix fields to WidgetEvent → Add LayersID field to WidgetEvent
Attachment #9044645 - Attachment description: Bug 1524226 - Add LayersId and matrix fields to WidgetEvent. → Bug 1524226 - Add LayersId field to WidgetEvent and InputData.

Comment 9

3 months ago
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/758cf7b85e44
Add LayersId field to WidgetEvent and InputData. r=masayuki

Comment 10

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Updated

3 months ago
Fission Milestone: --- → M1
Component: Event Handling → User events and focus handling
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.