Closed Bug 1518908 Opened 2 years ago Closed 4 months ago

Make sure that PresShell::GetResolution() works for oop-frames

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Fission Milestone M6c

People

(Reporter: jwatt, Assigned: botond)

References

(Blocks 3 open bugs, )

Details

(Whiteboard: [apz:fission:7:S])

Various bits of code look up the root presContext's presShell and call GetResolution() on it. For example, APZCCallbackHelper::ApplyCallbackTransform does this to “scale [the passed point] inversely [to undo] the scale-to-resolution transform that the compositor adds to the layer”.

Keeping this resolution in sync across oop-frames still requires some thought. Zooming followed by a tap, for example, may end up sending events to the wrong place. Kats had some thoughts on this.

needinfo is just to make sure you're aware of this bug in case it duplicates anything gfx is working on.

Flags: needinfo?(rhunt)

Botond, I understand you're working on pinch zoom for desktop. Just needinfo'ing you so you're aware of this in case it's covered elsewhere.

Flags: needinfo?(botond)
Flags: needinfo?(rhunt)

We don't need this for desktop zooming itself, but we'll need it for the combination of desktop zooming + fission. I'll keep it in mind.

Flags: needinfo?(botond)
Blocks: 1623473
Fission Milestone: --- → M7
Whiteboard: [apz:fission:7:S]

This will affect pinch zooming which is shipping already, so M6c.

Assignee: nobody → botond
Status: NEW → ASSIGNED
Fission Milestone: M7 → M6c

I think we can close this for now.

A suspicious place we thought we need to change was ViewportUtils::GetVisualToLayoutTransform, but it turns out we don't need it as I commented in bug 1649447 comment 12.

I did skim all call sites quickly, most of them look it should work as it is (I am not sure about accessibility stuff though).

Anyway, a basic principal fact is that we should handle the desktop zooming value via BrowserChild::mChildToParentConversionMatrix for OOP iframe documents if it's necessary. (Most of the call sites of GetResolution() don't need it though as far as I can tell) That means we need to change the call sites instead of GetResolution().

CCing Jamie. Jamie if you noticed any issue related to PresShell::GetResolution() calls in accessibility stuff (I am putting a searchfox link for all call sites of the function), please open a new bug, I can probably help to fix the issue. Thanks!

Closing.

Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.