Commit c68b078d authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

[LayoutNG] Cannot re-use line box fragments when scrollbars change.

Without this fix, 3 tests would fail with
https://chromium-review.googlesource.com/c/chromium/src/+/1297367 :

external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-paint-clip-004.html
fast/overflow/image-selection-highlight.html
scrollbars/overflow-scrollbar-combinations.html

Change-Id: Iabd95ead6c96cd50f25e654dda6a7f041ac77af0
Reviewed-on: https://chromium-review.googlesource.com/c/1311926Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604883}
parent 773a6109
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS22/visufx.html#propdef-overflow">
<style>
/* Avoid auto scrollbars on the viewport, because that might trigger re-layout
(and thus hide bugs). */
body { overflow:hidden; }
.container { width:100px; }
</style>
<p>There should be a blue rectangle below, and possibly a scrollbar (depending
on OS / browser), that shouldn not obscure any parts of the rectangle. The
word "FAIL" should not be seen.</p>
<div class="container" style="overflow:auto; height:200px;">
<div id="child" style="display:inline-block; box-sizing:border-box; width:100%; height:100%; border:10px solid blue;"></div>
<br>FAIL
</div>
<div class="container" style="visibility:hidden; overflow:scroll;">
<div id="ref"></div>
</div>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
test(()=> {
var child = document.getElementById("child");
assert_equals(child.offsetWidth, ref.offsetWidth);
}, "Auto scrollbar affects size of children");
</script>
...@@ -280,10 +280,6 @@ bool LayoutNGMixin<Base>::AreCachedLinesValidFor( ...@@ -280,10 +280,6 @@ bool LayoutNGMixin<Base>::AreCachedLinesValidFor(
*Base::cached_constraint_space_; *Base::cached_constraint_space_;
DCHECK(cached_result_); DCHECK(cached_result_);
// When scrollbar changes, |constraint_space.AvailableSize()| becomes
// different without setting |NeedsLayout()|.
// TODO(kojii): Should NGBlockNode::Layout() set NeedsLayout() when scrollbar
// change was detected and needs to relayout?
if (constraint_space.AvailableSize().inline_size != if (constraint_space.AvailableSize().inline_size !=
cached_constraint_space.AvailableSize().inline_size) cached_constraint_space.AvailableSize().inline_size)
return false; return false;
......
...@@ -252,6 +252,12 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout( ...@@ -252,6 +252,12 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
// and a child gained a vertical scrollbar. However, no test fails // and a child gained a vertical scrollbar. However, no test fails
// without that check. // without that check.
PaintLayerScrollableArea::FreezeScrollbarsScope freeze_scrollbars; PaintLayerScrollableArea::FreezeScrollbarsScope freeze_scrollbars;
// Scrollbar changes are hard to detect. Make sure everyone gets the
// message.
box_->SetNeedsLayout(LayoutInvalidationReason::kScrollbarChanged,
kMarkOnlyThis);
layout_result = LayoutWithAlgorithm(*this, constraint_space, break_token, layout_result = LayoutWithAlgorithm(*this, constraint_space, break_token,
/* ignored */ nullptr); /* ignored */ nullptr);
FinishLayout(block_flow, constraint_space, break_token, layout_result); FinishLayout(block_flow, constraint_space, break_token, layout_result);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment