Bug 1738781 - Invalidate anchor after layout if we can find a better one. r=dholbert

This is _very_ similar to code Blink has dealing with
content-visibility: auto:

  https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/layout/scroll_anchor.cc;l=658-659;drc=fd8802b593110ea18a97ef044f8a40dd24a622ec

They don't have this problem on the original test-case because they
incorrectly invalidate the anchor when focus changes regardless of
whether the focus is a "priority element" per spec.

So I think this is a better, more consistent behavior over all.

Differential Revision: https://phabricator.services.mozilla.com/D130130
This commit is contained in:
Emilio Cobos Álvarez 2021-11-02 10:59:50 +00:00
Родитель 3f2c1a3ad6
Коммит d0295b6c6f
4 изменённых файлов: 80 добавлений и 0 удалений

Просмотреть файл

@ -7,6 +7,7 @@
#include "ScrollAnchorContainer.h"
#include "mozilla/dom/Text.h"
#include "mozilla/ScopeExit.h"
#include "mozilla/PresShell.h"
#include "mozilla/ProfilerLabels.h"
#include "mozilla/StaticPrefs_layout.h"
@ -45,6 +46,7 @@ ScrollAnchorContainer::ScrollAnchorContainer(ScrollFrameHelper* aScrollFrame)
mAnchorNode(nullptr),
mLastAnchorOffset(0),
mDisabled(false),
mAnchorMightBeSubOptimal(false),
mAnchorNodeIsDirty(true),
mApplyingAnchorAdjustment(false),
mSuppressAnchorAdjustment(false) {}
@ -263,6 +265,8 @@ void ScrollAnchorContainer::SelectAnchor() {
ANCHOR_LOG("Skipping selection, doesn't maintain a scroll anchor.\n");
mAnchorNode = nullptr;
}
mAnchorMightBeSubOptimal =
mAnchorNode && mAnchorNode->HasAnyStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);
// Update the anchor flags if needed
if (oldAnchor != mAnchorNode) {
@ -400,6 +404,7 @@ void ScrollAnchorContainer::InvalidateAnchor(ScheduleSelection aSchedule) {
FindFor(Frame())->InvalidateAnchor();
}
mAnchorNode = nullptr;
mAnchorMightBeSubOptimal = false;
mAnchorNodeIsDirty = true;
mLastAnchorOffset = 0;
@ -445,6 +450,16 @@ void ScrollAnchorContainer::ApplyAdjustments() {
ANCHOR_LOG("Anchor has moved from %d to %d.\n", mLastAnchorOffset, current);
auto maybeInvalidate = MakeScopeExit([&] {
if (mAnchorMightBeSubOptimal &&
StaticPrefs::layout_css_scroll_anchoring_reselect_if_suboptimal()) {
ANCHOR_LOG(
"Anchor might be suboptimal, invalidating to try finding a better "
"one\n");
InvalidateAnchor();
}
});
if (logicalAdjustment == 0) {
ANCHOR_LOG("Ignoring zero delta anchor adjustment for %p.\n", this);
mSuppressAnchorAdjustment = false;

Просмотреть файл

@ -167,6 +167,9 @@ class ScrollAnchorContainer final {
// layout.css.scroll-anchoring.min-adjustment-threshold.
bool mDisabled : 1;
// True if when we selected the current scroll anchor, there were unlaid out
// children that could be better anchor nodes after layout.
bool mAnchorMightBeSubOptimal : 1;
// True if we should recalculate our anchor node at the next chance
bool mAnchorNodeIsDirty : 1;
// True if we are applying a scroll anchor adjustment

Просмотреть файл

@ -7596,6 +7596,14 @@
value: false
mirror: always
# Pref to control whether we reselect scroll anchors if sub-optimal
#
# See https://github.com/w3c/csswg-drafts/issues/6787
- name: layout.css.scroll-anchoring.reselect-if-suboptimal
type: bool
value: true
mirror: always
# Are shared memory User Agent style sheets enabled?
- name: layout.css.shared-memory-ua-sheets.enabled
type: bool

Просмотреть файл

@ -0,0 +1,54 @@
<!doctype html>
<meta charset=utf-8>
<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1738781">
<link rel=help href="https://github.com/w3c/csswg-drafts/issues/6787">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
.padding {
background: grey;
border: 1px dashed black;
margin: 5px;
height: 200vh;
}
</style>
<div id="content"></div>
<script>
const content = document.getElementById("content");
const t = async_test("Scroll anchor is re-selected after adjustment if there are dirty descendants at selection time");
function replaceAllContent() {
content.innerHTML = `
<div class="padding"></div>
<button id="target">Scroll target</button>
<div class="padding"></div>
`;
}
function insertContent() {
let inserted = document.createElement("div");
inserted.className = "padding inserted";
content.insertBefore(inserted, content.firstChild);
}
// Set the content, and scroll #target into view.
replaceAllContent();
document.getElementById("target").scrollIntoView();
t.step(function() {
assert_not_equals(window.scrollY, 0, "Should've scrolled");
});
// Save the target scroll position, which shouldn't change.
const oldTargetTop = document.getElementById("target").getBoundingClientRect().top;
// Replace all the content, then insert content at the top afterwards.
replaceAllContent();
requestAnimationFrame(() => requestAnimationFrame(t.step_func_done(function() {
insertContent();
const newTargetTop = document.getElementById("target").getBoundingClientRect().top;
assert_equals(oldTargetTop, newTargetTop, "Scroll position should've been preserved");
})));
</script>
<style>