Skip to content

Commit

Permalink
[GridNG] Fix querying %-padding on grid-items.
Browse files Browse the repository at this point in the history
As above. ComputedCSSPadding will simply look at the available
inline-size of its containing-block. Legacy grid worked by setting the
override containing-block inline-size.

This patch writes back the %-size.

This isn't ideal - in a future world we should answer these queries
directly from the fragment(s), but we aren't there yet.

Bug: 1045599
Change-Id: I3afd8d03b0410dd0e02aba4133de28d92432630e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2721043
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858396}
  • Loading branch information
bfgeek authored and Chromium LUCI CQ committed Feb 27, 2021
1 parent b7fc06f commit 0f56b3a
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 15 deletions.
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc
Expand Up @@ -318,7 +318,7 @@ void LayoutNGMixin<Base>::UpdateOutOfFlowBlockLayout() {
// These are the unpositioned OOF descendants of the current OOF block.
for (const auto& descendant :
result->PhysicalFragment().OutOfFlowPositionedDescendants())
descendant.node.UseLegacyOutOfFlowPositioning();
descendant.node.InsertIntoLegacyPositionedObjects();

const auto& fragment = result->PhysicalFragment();
DCHECK_GT(fragment.Children().size(), 0u);
Expand Down Expand Up @@ -366,7 +366,7 @@ LayoutNGMixin<Base>::UpdateInFlowBlockLayout() {

for (const auto& descendant :
physical_fragment.OutOfFlowPositionedDescendants())
descendant.node.UseLegacyOutOfFlowPositioning();
descendant.node.InsertIntoLegacyPositionedObjects();

// Even if we are a layout root, our baseline may have shifted. In this
// (rare) case, mark our containing-block for layout.
Expand Down
11 changes: 10 additions & 1 deletion third_party/blink/renderer/core/layout/ng/ng_block_node.cc
Expand Up @@ -1076,6 +1076,15 @@ void NGBlockNode::CopyFragmentDataToLayoutBox(
box_->SetMargin(ComputePhysicalMargins(constraint_space, Style()));
}

// We copy back the %-size so that |LayoutBoxModelObject::ComputedCSSPadding|
// is able to return the correct value. This isn't ideal, but eventually
// we'll answer these queries from the fragment.
const auto* containing_block = box_->ContainingBlock();
if (UNLIKELY(containing_block && containing_block->IsLayoutNGGrid())) {
box_->SetOverrideContainingBlockContentLogicalWidth(
constraint_space.PercentageResolutionInlineSizeForParentWritingMode());
}

auto* block_flow = DynamicTo<LayoutBlockFlow>(box_);
LayoutMultiColumnFlowThread* flow_thread = GetFlowThread(block_flow);

Expand Down Expand Up @@ -1770,7 +1779,7 @@ void NGBlockNode::UpdateShapeOutsideInfoIfNeeded(
percentage_resolution_inline_size);
}

void NGBlockNode::UseLegacyOutOfFlowPositioning() const {
void NGBlockNode::InsertIntoLegacyPositionedObjects() const {
DCHECK(box_->IsOutOfFlowPositioned());
box_->ContainingBlock()->InsertPositionedObject(box_);
}
Expand Down
4 changes: 1 addition & 3 deletions third_party/blink/renderer/core/layout/ng/ng_block_node.h
Expand Up @@ -163,9 +163,7 @@ class CORE_EXPORT NGBlockNode : public NGLayoutInputNode {
NGBaselineAlgorithmType baseline_algorithm_type =
NGBaselineAlgorithmType::kInlineBlock);

// Called if this is an out-of-flow block which needs to be
// positioned with legacy layout.
void UseLegacyOutOfFlowPositioning() const;
void InsertIntoLegacyPositionedObjects() const;

// Write back resolved margins to legacy.
void StoreMargins(const NGConstraintSpace&, const NGBoxStrut& margins);
Expand Down
Expand Up @@ -532,14 +532,14 @@ void NGOutOfFlowLayoutPart::LayoutCandidates(
container_builder_->AddOutOfFlowFragmentainerDescendant(candidate);
continue;
}
if (layout_box != only_layout)
candidate.node.InsertIntoLegacyPositionedObjects();
scoped_refptr<const NGLayoutResult> result =
LayoutOOFNode(SetUpNodeForLayout(candidate), only_layout);
container_builder_->AddChild(result->PhysicalFragment(),
result->OutOfFlowPositionedOffset(),
candidate.inline_container);
placed_objects->insert(candidate.node.GetLayoutBox());
if (layout_box != only_layout)
candidate.node.UseLegacyOutOfFlowPositioning();
} else {
container_builder_->AddOutOfFlowDescendant(candidate);
}
Expand Down
7 changes: 0 additions & 7 deletions third_party/blink/web_tests/TestExpectations
Expand Up @@ -3632,12 +3632,6 @@ crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/gr
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-margins-vertical-lr-002.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-margins-vertical-rl-001.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-margins-vertical-rl-002.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-paddings-001.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-paddings-002.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-001.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-001.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-items-relative-offsets-002.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-021.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-022.html [ Failure ]
Expand Down Expand Up @@ -3723,7 +3717,6 @@ crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/named-grid-lines-c
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/negative-growth-share-as-infinity-crash.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/non-grid-columns-rows-get-set.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/percent-padding-margin-resolution-grid-item-update.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/percent-track-breadths-regarding-container-size.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/positioned-grid-container-percentage-tracks.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/fast/css-grid-layout/relayout-indefinite-heights.html [ Failure ]
Expand Down
@@ -0,0 +1,13 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-grid-1/#grid-item-sizing">
<meta name="assert" content="Checks that the %-padding accessed from script, for an OOF descendant resolves correctly.">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/check-layout-th.js"></script>
<body onload="checkLayout('#target')">
<div style="display: inline-grid; position: relative; grid-template: 50px 20px / 50px 20px;">
<div style="grid-area: 2/2/2/2;">
<div id="target" style="position: absolute; top: 0; left: 0; grid-area: 1/1/1/1; padding: 100%; background: green;" data-expected-padding-left="50" data-expected-padding-top="50"></div>
</div>
</div>
</div>

0 comments on commit 0f56b3a

Please sign in to comment.