[LayoutNG] abspos applied to ::before are not invalidated properly |
||||
Issue descriptionFrom the dogfood experience, icons before trybots in Gerrit are positioned incorrectly. It looks like they have 'position: absolute' to '::before' pseudo elements. They go back to the correct position if I change some properties in Inspector, so this is probably an invalidation problem. atotic@, do you have bandwidth to have a look?
,
Jul 23
Trying to create a minimized case but failing. fast/box-sizing/replaced.html looks like invalidation problem with abspos. Maybe related? It seems to pass in trybots for [1] but I can't explain any reasons to fix this test. Maybe flaky? [1] https://chromium-review.googlesource.com/c/chromium/src/+/1146125
,
Aug 2
,
Aug 13
This has been bugging me too, and I finally realized that it's 100% reproducible: Open a review, scroll to the trybots. Click on the expansion button next to one of the above files to review. That takes up space, and pushes the trybots block down, except for the icons, which remain where they were. Attaching test. What seems to cause problems is when we have an auto-positioned out-of-flow positioned object inside a legacy layout subtree (in the test I use tables, but could have used flexbox as well, which is what gerrit uses), and then add or remove content before the legacy subtree. Then the out-of-flow objects remain put, rather than tagging along with the rest of the legacy subtree. But I'm not sure. I haven't debugged it.
,
Aug 14
@mstensho Did you forget to attach the test?
,
Aug 21
,
Aug 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/110ced38f55e5bd44b4b40c2b9dcde510b75a51a commit 110ced38f55e5bd44b4b40c2b9dcde510b75a51a Author: Aleks Totic <atotic@chromium.org> Date: Tue Aug 28 18:20:00 2018 [LayoutNG] Fix legacy initiated abspos invalidation Bug: LayoutBlock::LayoutPositionedObject had a static position check whether child reflow was necessary: if (.... || (!IsLayoutNGBlockFlow() && NeedsLayoutDueToStaticPosition(positioned_object)))) layout_scope.SetChildNeedsLayout(positioned_object); This check would skip reflow for all NGBlockFlow children. This is incorrect. If child abspos layout was initiated by Legacy, we should check static position. Fix: There was no way to tell in current code whether child abspos layout was initiated by legacy. Added a flag is_legacy_initiated_out_of_flow_layout_. Flag was added to LayoutBlock per ikilpatrick recommendation. Flag is set to true in LayoutNGBlockFlow::UpdateOutOfFlowBlockLayout Flag is set to false in NGOutOfFlowLayout. With this fix, checkboxes on code review page are correct. Bug: 863865 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Ifc55ac03b76dd1aa2113181681bca1b5a2832fb2 Reviewed-on: https://chromium-review.googlesource.com/1190723 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#586791} [add] https://crrev.com/110ced38f55e5bd44b4b40c2b9dcde510b75a51a/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/abspos/static-inside-table-cell.html [modify] https://crrev.com/110ced38f55e5bd44b4b40c2b9dcde510b75a51a/third_party/blink/renderer/core/layout/layout_block.cc [modify] https://crrev.com/110ced38f55e5bd44b4b40c2b9dcde510b75a51a/third_party/blink/renderer/core/layout/layout_block.h [modify] https://crrev.com/110ced38f55e5bd44b4b40c2b9dcde510b75a51a/third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.cc [modify] https://crrev.com/110ced38f55e5bd44b4b40c2b9dcde510b75a51a/third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc
,
Aug 29
|
||||
►
Sign in to add a comment |
||||
Comment 1 by kojii@chromium.org
, Jul 16