Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gecko Bug 1303605] Make LazyFC assertions actually hold. #10370

Merged
merged 1 commit into from Apr 9, 2018

Conversation

Projects
None yet
4 participants
@moz-wptsync-bot
Copy link
Collaborator

moz-wptsync-bot commented Apr 9, 2018

The code was trying to assert that we had frames constructed for all the nodes
in the parent chain, but we don't bail out in the
!GetContentInsertionFrameFor(aContainer) in the case that it's a children
element, because they actually have no insertion frame, though their children
do.

Move the LazyFC check after the insertion point check. That makes the previous
check work on the insertion point of the child, which makes it sound.

This also fixes bug 1410020, and with it a Shadow DOM test-case that was failing
because we had two sibling assigned to two different s, and the second one
wasn't getting properly flagged, and thus the second sibling never got a frame.

The other two test failures in this test are an event dispatch failure, where
the position of the target is not what the test expects (we don't account for
margin and padding). Filed that as bug 1450027.

Also, added a test for which we have wrong layout without these patches, and
that crashes with "Called Servo_Element_IsDisplayNone" with the first patch of
this bug applied but not this one, due to the bogus check mentioned above.
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1303605
gecko-commit: 12a824f8d55a8fb0396fb2132974f8223c6a9606
gecko-integration-branch: central
gecko-reviewers: bz


This change is Reviewable

Make LazyFC assertions actually hold.
The code was trying to assert that we had frames constructed for all the nodes
in the parent chain, but we don't bail out in the
!GetContentInsertionFrameFor(aContainer) in the case that it's a children
element, because they actually have no insertion frame, though their children
do.

Move the LazyFC check after the insertion point check. That makes the previous
check work on the insertion point of the child, which makes it sound.

This also fixes bug 1410020, and with it a Shadow DOM test-case that was failing
because we had two sibling assigned to two different <slot>s, and the second one
wasn't getting properly flagged, and thus the second sibling never got a frame.

The other two test failures in this test are an event dispatch failure, where
the position of the target is not what the test expects (we don't account for
margin and padding). Filed that as bug 1450027.

Also, added a test for which we have wrong layout without these patches, and
that crashes with "Called Servo_Element_IsDisplayNone" with the first patch of
this bug applied but not this one, due to the bogus check mentioned above.
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1303605
gecko-commit: 12a824f8d55a8fb0396fb2132974f8223c6a9606
gecko-integration-branch: central
gecko-reviewers: bz
@wpt-pr-bot
Copy link
Collaborator

wpt-pr-bot left a comment

Already reviewed downstream.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Apr 9, 2018

Build PENDING

Started: None
Finished: None

View more information about this build on:

@moz-wptsync-bot moz-wptsync-bot merged commit fc7c0b7 into master Apr 9, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
upstream/gecko Landed on mozilla-central
Details

@moz-wptsync-bot moz-wptsync-bot deleted the gecko/1303605 branch Apr 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.