NGLayoutInputNode::IsInline() is not deterministic |
|||
Issue descriptionWhen we have: <body> <div>a</div> <div>b</div> </body> and traverse children of <body>, the <div>s are block. However, when we traverse children of <div>s, it finds inline children and SetLayoutNGInline(true), but this LayoutObject is shared with parent NGBlockNode, so the parent NGBlockNode turns to NGInlineNode. For instance, next = <body>.FirstChild().NextSibling() returns 2nd <div>, but: child = <body>.FirstChild(); grand_child = child.FirstChild(); next = child.NextSibling(); returns nullptr, because |child| becomes IsInline() in the 2nd line. This problem is causing failures such as: virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-valign-001.xht virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-block-valign-002.xht because we use NGBlockChildIterator to layout, which calls NextSibiling() earlier than looking into grand children, while ComputeMinMaxContentSize() uses FirstChild/NextSibling to iterate children, which calls NextSibling after grand children are traversed, and fails to find 2nd <div>. This symptom can be fixed by ComputeMinMaxContentSize() to use NGBlockChildIterator: https://codereview.chromium.org/2970783002 but I guess this issue should be fixed too. Not finding a good way to fix yet though. Thoughts?
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/417883d42a4de932d08ce17e00822bab34ab72f0 commit 417883d42a4de932d08ce17e00822bab34ab72f0 Author: Koji Ishii <kojii@chromium.org> Date: Wed Jul 12 04:33:19 2017 [LayoutNG] Fix NGLayoutInputNode::IsInline/IsBlock Currently, NGLayoutInputNode::IsInline() uses a bit field in LayoutObject. This can mis-behave when NGBlockNode and NGInlineNode share a LayoutNGBlockFLow. This patch adds a type field back to NGLayoutInputNode. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng Bug: 739087 Change-Id: I137ee5a5233d96fbb3297f1054f2cab7e4c06d33 Reviewed-on: https://chromium-review.googlesource.com/566984 Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#485863} [modify] https://crrev.com/417883d42a4de932d08ce17e00822bab34ab72f0/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/417883d42a4de932d08ce17e00822bab34ab72f0/third_party/WebKit/Source/core/layout/LayoutObject.h [modify] https://crrev.com/417883d42a4de932d08ce17e00822bab34ab72f0/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc [modify] https://crrev.com/417883d42a4de932d08ce17e00822bab34ab72f0/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc [modify] https://crrev.com/417883d42a4de932d08ce17e00822bab34ab72f0/third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.cc [modify] https://crrev.com/417883d42a4de932d08ce17e00822bab34ab72f0/third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.h
,
Jul 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f18e72903c351dddfe064f071eab9b2486146554 commit f18e72903c351dddfe064f071eab9b2486146554 Author: Koji Ishii <kojii@chromium.org> Date: Thu Jul 13 05:40:16 2017 [LayoutNG] Fix percentage of 'text-indent' to use containing block The spec[1] defines percentage of 'text-indent' uses the containing block of the block container. To match this definition, this patch adds ParentPercentageResolutionInlineSize to NGConstraintSpace. The parent constraint space can serve the purpose since it's the space for the containing block, including the case for positioned objects. One missing piece in this CL is that ComputeMinMaxContentSize() does not know the parent constraint space, and that it fails when 'text-indent' is applied to positioned objects. Supporting this is to be discussed further. All impls except Gecko support this behavior. [1] https://drafts.csswg.org/css-text-3/#valdef-text-indent-percentage Bug: 739087 Change-Id: I3a7fea14e2e97c45d9416388f355d7d479b595f7 Reviewed-on: https://chromium-review.googlesource.com/568559 Commit-Queue: Koji Ishii <kojii@chromium.org> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#486284} [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/Source/core/layout/ng/geometry/ng_logical_size.h [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_item_result.cc [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc [modify] https://crrev.com/f18e72903c351dddfe064f071eab9b2486146554/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h
,
Jul 13 2017
,
Jul 26 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by kojii@chromium.org
, Jul 11 2017