1950303 - 'contain:layout' should prevent wide abspos elements from inflating the mobile viewport

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Load attached testcase on Android or in RDM on Desktop.
2. Try to scroll (horizontally or vertically).

ACTUAL RESULTS:
I can scroll.

EXPECTED RESULTS:
Should not be able to scroll.

There's some overflow contributed by a large abspos element. That element is inside of a contain:layout wrapper, which I think is supposed to prevent that element from actually generating scrollable overflow.

Comment 1

[Daniel Holbert [:dholbert]]

Actually, the same bug happens if I just load the testcase directly on desktop (no need for RDM). Firefox shows scrollbars and will let you scroll the page; Chrome and WebKit do not.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

So in the case of comment 0, the abspos element runs through:

https://searchfox.org/firefox-main/rev/69a6da6b2f5f8f6bdb90cd6dd70a024f460f12ca/layout/generic/AbsoluteContainingBlock.cpp#2271-2272

aOverflowAreas->UnionWithAbsoluteOverflowAreas(
    aKidFrame->GetOverflowAreasRelativeToParent());

this code path in AbsoluteConainingBlock::ReflowAbsoluteFrame, rather than

https://searchfox.org/firefox-main/rev/69a6da6b2f5f8f6bdb90cd6dd70a024f460f12ca/layout/generic/AbsoluteContainingBlock.cpp#886

aDelegatingFrame->ConsiderChildOverflow(*aOverflowAreas, kidFrame);

this code path in AbsoluteContainingBlock::Reflow. Thus, this special handling for contain:layout is not applied to:

https://searchfox.org/firefox-main/rev/69a6da6b2f5f8f6bdb90cd6dd70a024f460f12ca/layout/generic/nsContainerFrame.cpp#2427-2437

if (StyleDisplay()->IsContainLayout() && SupportsContainLayoutAndPaint() &&
    !aAsIfScrolled) {
  // If we have layout containment and are not a non-atomic, inline-level
  // principal box, we should only consider our child's ink overflow,
  // leaving the scrollable regions of the parent unaffected.
  // Note: scrollable overflow is a subset of ink overflow,
  // so this has the same affect as unioning the child's ink and
  // scrollable overflow with the parent's ink overflow.
  const OverflowAreas childOverflows(aChildFrame->InkOverflowRect(),
                                     nsRect());
  aOverflowAreas.UnionWith(childOverflows + aChildFrame->GetPosition());

Adding a similar check in AbsoluteConainingBlock::ReflowAbsoluteFrame solves this bug.

That's being said, I am not sure whether the place where ConsiderChildOverflow is called in AbsoluteContainingBlock::Reflow should also use UnionWithAbsoluteOverflowAreas instead of UnionWith. I have tried to create a similar test case to the one in bug 2025540 which introduced UnionWithAbsoluteOverflowAreas, but the attempt was not successful.

So I am going to leave the ConsiderChildOverflow call as it is.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1950303 - Apply contain:layout overflow handling to absolutely positioned children in AbsoluteContainingBlock::ReflowAbsoluteFrame. r?#layout-reviewers

When a containing block has contain: layout, scrollable overflow from its
children should not propagate to the parent. nsContainerFrame::ConsiderChildOverflow
already implements this by only unioning the child's ink overflow, but
AbsoluteContainingBlock::ReflowAbsoluteFrame took a separate code path that
unconditionally unioned the child's full overflow areas. As a result, a wide
absolutely positioned descendant of a contain: layout element could still
inflate the page's scrollable area (and the mobile viewport).

Mirror the contain:layout handling from nsContainerFrame::ConsiderChildOverflow
here so that only the child's ink overflow contributes when the delegating
frame has layout containment.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1950303 - Convert nsContainerFrame::ConsiderChildOverflow's bool parameter to OverflowAreaUnionFlags enum. r?#layout-reviewers

This is a preparatory refactor for a follow-up commit that introduces a new
flag (for absolutely positioned children). Using a bit-flag enum makes call
sites self-documenting and lets us add more orthogonal options without
expanding the parameter list.

No behavior change.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b42414c3c030
https://hg.mozilla.org/integration/autoland/rev/2718e04254c2
Convert nsContainerFrame::ConsiderChildOverflow's bool parameter to OverflowAreaUnionFlags enum. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/8851ed32ff7d
https://hg.mozilla.org/integration/autoland/rev/df1a015a45f4
Apply contain:layout overflow handling to absolutely positioned children in AbsoluteContainingBlock::ReflowAbsoluteFrame. r=layout-reviewers,emilio

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/59808 for changes under testing/web-platform/tests

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2718e04254c2
https://hg.mozilla.org/mozilla-central/rev/df1a015a45f4

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot