Skip to content

GitLab

T
tangled
Commit 2407cec6 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot
Browse files
Options

[layout] Don't freeze OOF-positioned scrollbars within flex layout. 

Change:
https://chromium-review.googlesource.com/c/chromium/src/+/2424899
... introdcued a fix for freeze more consistent scrollbars for
arbitrary descendants of flex layout. However the original code
(detected direct descendant scrollbar changes) contained an issue that
it would also freeze scrollbars for any OOF positioned children.

There exists a wider issue which I've detailed here:
https://bugs.chromium.org/p/chromium/issues/detail?id=1148835
But it's relative rare.

The fix for this specific regression is to unfreeze the scrollbars
before running the OOF layout part.

Bug: 1148288
Change-Id: I74be0bf141d13988548d38412e912c8c98d628b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538041Reviewed-by: default avatarDavid Grogan <dgrogan@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827402}
parent 73bccc9d
Hide whitespace changes
Inline Side-by-side
Showing with 29 additions and 3 deletions
third_party/blink/renderer/core/layout/ng/flex/ng_flex_layout_algorithm.cc
View file @ 2407cec6
...@@ -1022,9 +1022,6 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() { ...@@ -1022,9 +1022,6 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() {
scoped_refptr<const NGLayoutResult> scoped_refptr<const NGLayoutResult>
NGFlexLayoutAlgorithm::RelayoutIgnoringChildScrollbarChanges() { NGFlexLayoutAlgorithm::RelayoutIgnoringChildScrollbarChanges() {
DCHECK(!ignore_child_scrollbar_changes_); DCHECK(!ignore_child_scrollbar_changes_);
// Freezing the scrollbars for the sub-tree shouldn't be strictly necessary,
// but we do this just in case we trigger an unstable layout.
PaintLayerScrollableArea::FreezeScrollbarsScope freeze_scrollbars;
NGLayoutAlgorithmParams params( NGLayoutAlgorithmParams params(
Node(), container_builder_.InitialFragmentGeometry(), ConstraintSpace(), Node(), container_builder_.InitialFragmentGeometry(), ConstraintSpace(),
BreakToken(), /* early_break */ nullptr); BreakToken(), /* early_break */ nullptr);
...@@ -1034,6 +1031,13 @@ NGFlexLayoutAlgorithm::RelayoutIgnoringChildScrollbarChanges() { ...@@ -1034,6 +1031,13 @@ NGFlexLayoutAlgorithm::RelayoutIgnoringChildScrollbarChanges() {
} }
scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::LayoutInternal() { scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::LayoutInternal() {
// Freezing the scrollbars for the sub-tree shouldn't be strictly necessary,
// but we do this just in case we trigger an unstable layout.
base::Optional<PaintLayerScrollableArea::FreezeScrollbarsScope>
freeze_scrollbars;
if (ignore_child_scrollbar_changes_)
freeze_scrollbars.emplace();
PaintLayerScrollableArea::DelayScrollOffsetClampScope delay_clamp_scope; PaintLayerScrollableArea::DelayScrollOffsetClampScope delay_clamp_scope;
ConstructAndAppendFlexItems(); ConstructAndAppendFlexItems();
...@@ -1211,6 +1215,8 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::LayoutInternal() { ...@@ -1211,6 +1215,8 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::LayoutInternal() {
if (!success) if (!success)
return nullptr; return nullptr;
// Un-freeze descendant scrollbars before we run the OOF layout part.
freeze_scrollbars.reset();
NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run(); NGOutOfFlowLayoutPart(Node(), ConstraintSpace(), &container_builder_).Run();
return container_builder_.ToBoxFragment(); return container_builder_.ToBoxFragment();
......
third_party/blink/web_tests/external/wpt/css/css-flexbox/position-absolute-scrollbar-freeze-ref.html 0 → 100644
View file @ 2407cec6
<!DOCTYPE html>
<div style="width: 100px; height: 100px; overflow: auto;">
<div style="height: 200px; background: green;"></div>
</div>
third_party/blink/web_tests/external/wpt/css/css-flexbox/position-absolute-scrollbar-freeze.html 0 → 100644
View file @ 2407cec6
<!DOCTYPE html>
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1148288">
<link rel="match" href="position-absolute-scrollbar-freeze-ref.html">
<div style="display: flex; position: relative;">
<div style="display: flex;">
<div id="target1" style="width: 0px;">text</div>
</div>
<div id="target2" style="position: absolute; overflow: auto; top: 0; left: 0; width: 100px; max-height: 100px; display: none;">
<div style="height: 200px; background: green;"></div>
</div>
</div>
<script>
document.body.offsetTop;
document.getElementById('target1').innerText = '';
document.getElementById('target2').style.display = '';
</script>
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