[layout] Merge anonymous objects more correctly. #56781
Open
+13
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously we had ad-hoc logic within LayoutBlockFlow and LayoutFlexibleBox to merge (some) anonymous objects. This logic was incomplete, and didn't work for all types which needed this behaviour (e.g. LayoutGrid). A complex case for this patch is: ``` <div style="display: table-cell;">1</div> <span id="remove"></span> <div style="display: table-cell;">2</div> <script>remove.remove();</script> ``` Before removal the table-cells are each within their own anonymous table, section, and row. After removal, they should be within the same table, section, and row. To do this we attempt to merge objects directly after an object has been removed from LayoutObjectChildList. Objects are merged if they they are anonymous, and compatible with the other object (e.g. the same type typically). This check is implemented as LayoutBoxModelObject::CanMergeWith. Document destruction is a useful stress-test of this new code as removing objects during this phase tests more of the merging code. As such - this patch depends on the DisableDocumentBeingDestroyed feature. All behaviour changes should be behind the flag: LayoutMergeAnonymousFix Bug: 468061895 Change-Id: Ic9ac47529509b65943481e114f563ed535c849f8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7244131 Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/main@{#1559408}
wpt-pr-bot
approved these changes
Dec 16, 2025
Collaborator
wpt-pr-bot
left a comment
wpt-pr-bot
left a comment
There was a problem hiding this comment.
The review process for this patch is being conducted in the Chromium project.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we had ad-hoc logic within LayoutBlockFlow and
LayoutFlexibleBox to merge (some) anonymous objects. This logic was
incomplete, and didn't work for all types which needed this behaviour
(e.g. LayoutGrid).
A complex case for this patch is:
Before removal the table-cells are each within their own anonymous
table, section, and row. After removal, they should be within the same
table, section, and row.
To do this we attempt to merge objects directly after an object has been
removed from LayoutObjectChildList.
Objects are merged if they they are anonymous, and compatible with the
other object (e.g. the same type typically).
This check is implemented as LayoutBoxModelObject::CanMergeWith.
Document destruction is a useful stress-test of this new code as
removing objects during this phase tests more of the merging code.
As such - this patch depends on the DisableDocumentBeingDestroyed
feature.
All behaviour changes should be behind the flag:
LayoutMergeAnonymousFix
Bug: 468061895
Change-Id: Ic9ac47529509b65943481e114f563ed535c849f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7244131
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1559408}