Closed Bug 1535875 Opened 5 years ago Closed 5 years ago

[wpt-sync] Sync PR 15794 - [LayoutNG] Fix for DCHECK(NeedsLayout) for fixed

Categories

(Core :: CSS Parsing and Computation, enhancement, P4)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 15794 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/15794
Details from upstream follow.

Aleks Totic <atotic@chromium.org> wrote:

[LayoutNG] Fix for DCHECK(NeedsLayout) for fixed

All tests pass, and crashes no longer happen. I believe
that code will not longer crash, but there might be
futher instances of incorrect positioning.

Fix #1
LayoutDescendantCandidates did not sweep newly discovered
candidates. This was done
manually once inside NGOutOfFlowLayoutPart::Run, and
sweep was not performed for LayoutDescendantCandidates
found in Legacy. Fix is to make LayoutDescendantCandidates
perform sweep instead.

Fix #2
fix #1 exposed a bug where duplicate fragments were generated
for a single layout object. This happened when NG was generating
fragments not inside ContainingBlock. Fix one instance of this
inside NGOutOfFlowLayoutPart::IsContainingBlockForDescendant
by making sure that OOF with inline containers are only positioned
inside its ContainingBlock()

Fix #3
NGOutOfFlowLayoutPart::LayoutDescendant offset adjustment.
This one I am unsure about because I do not know exactly how
split inlines generate anonymous boxes.
if (container != container_builder_->GetLayoutObject() &&
container->Parent()->IsAnonymousBlock())
It extends the previus special case of css_container
being anonymous. It seems that it should be somehow generalized
to if "anything in ancestor chain to ContainerBlock() is anonymous?"

Bug: 935805
Change-Id: I9f7ebbc7223f40fbbf6ba3739d9385bfd59e3641
Reviewed-on: https://chromium-review.googlesource.com/1517093
WPT-Export-Revision: 95b1bcec6c5fec3604fae1e265ed3e78277efec6

PR 15794 applied with additional changes from upstream: 409e5448a0a690d5ea9c0f9be3fb75f638e0883f
Component: web-platform-tests → CSS Parsing and Computation
Product: Testing → Core
Whiteboard: [wptsync downstream] → [wptsync downstream error]
Whiteboard: [wptsync downstream error] → [wptsync downstream]
Failed to get results from try push
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cde4bc00951
[wpt PR 15794] - [LayoutNG] Fix for DCHECK(NeedsLayout) for fixed, a=testonly
https://hg.mozilla.org/integration/mozilla-inbound/rev/feaaf527c014
[wpt PR 15794] - Update wpt metadata, a=testonly
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19bb7a1ca6b5
[wpt PR 15794] - [LayoutNG] Fix for DCHECK(NeedsLayout) for fixed, a=testonly
https://hg.mozilla.org/integration/mozilla-inbound/rev/c59c26b4a95f
[wpt PR 15794] - Update wpt metadata, a=testonly
You need to log in before you can comment on or make changes to this bug.