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

Don't miss list item markers inside multicol containers. #28839

Merged
merged 1 commit into from May 11, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented May 5, 2021

With list-style-position:inside, we'd fail to find the marker if it was
inside a multicol container, because we'd look for it in the flow
thread, and give up, since the flow thread isn't the list-item (the
multicol container (the parent LayoutObject) is).

Flow threads should be ignored in LayoutNG. Use
GetLayoutObjectForParentNode() if we need to walk up the tree, to get
this right. Templatize GetLayoutObjectForParentNode(), so that it can
provide both a const and non-const version. CollectInlinesInternal()
works on non-const objects, and there doesn't seem to be an easy way
around that.

This fixes the final failures in multicol-span-all-list-item-001.html
and multicol-span-all-list-item-002.html, but since those tests were
quite complicated (and mostly about spanners, even if this CL has
nothing to do with spanners), I added a simpler test for this specific
issue.

Bug: 829028, 1130451
Change-Id: I6bcf10593a68c40cbc9adc11f9572c9aee580623
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2874650
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#881621}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

With list-style-position:inside, we'd fail to find the marker if it was
inside a multicol container, because we'd look for it in the flow
thread, and give up, since the flow thread isn't the list-item (the
multicol container (the parent LayoutObject) is).

Flow threads should be ignored in LayoutNG. Use
GetLayoutObjectForParentNode() if we need to walk up the tree, to get
this right. Templatize GetLayoutObjectForParentNode(), so that it can
provide both a const and non-const version. CollectInlinesInternal()
works on non-const objects, and there doesn't seem to be an easy way
around that.

This fixes the final failures in multicol-span-all-list-item-001.html
and multicol-span-all-list-item-002.html, but since those tests were
quite complicated (and mostly about spanners, even if this CL has
nothing to do with spanners), I added a simpler test for this specific
issue.

Bug: 829028, 1130451
Change-Id: I6bcf10593a68c40cbc9adc11f9572c9aee580623
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2874650
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#881621}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants