New issue
Advanced search Search tips

Issue 906260 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[LayoutNG] Leftover NGInlineNodeData on blocks that are previously inline formatting context

Project Member Reported by xiaoche...@chromium.org, Nov 16

Issue description

Version: ToT @ r608853

Say we have an inline formatting context, like:

<div id=target>foo</div>

There should be an NGInlineNodeData at the LayoutNGBlockFlow. Everything is fine so far.

However, if we turn #target into a block formatting context by, for example
- target.innerHTML = ""
- or target.innerHTML = "<div>foo</div>"

Then after relayout, the original NGInlineNodeData at #target is still there. It should be cleared since we shouldn't have NGInlineNodeData if a block isn't inline formatting context.

This currently doesn't cause any malfunctioning, since we don't access NGInlineNodeData from a block formatting context. Having a leftover data there is still bad...

This also causes failures in DocumentMarkerControllerTest, which asserts that text node is cleared by GC after detaching from DOM, which isn't due to the leftover NGInlineNodeData keeping reference to the node (through NGOffsetMapping).
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4adfdfd1e23761a0a6b671ef29437703789c7336

commit 4adfdfd1e23761a0a6b671ef29437703789c7336
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed Dec 05 19:51:31 2018

Remove NGInlineNodeData if a block flow is no longer inline formatting context

This patch removes NGInlineNodeData if a block flow is no longer inline
formatting context, so that we don't have a leftover on it.

Note: If a block becomes empty, its ChildrenInline flag remains true, so we
still need to remove it in NGBlockNode::Layout.

Bug: 906260
Change-Id: Id79d2c62da6a046bbd1f076b7a30d2ce39c095c7
Reviewed-on: https://chromium-review.googlesource.com/c/1362482
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614067}
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/editing/markers/document_marker_controller_test.cc
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/layout/layout_block_flow.cc
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/layout/layout_block_flow.h
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.h
[modify] https://crrev.com/4adfdfd1e23761a0a6b671ef29437703789c7336/third_party/blink/renderer/core/layout/ng/ng_block_node.cc

Sign in to add a comment