Commit d3da1b8b authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

SubtreeVisibility: don't activate on scrollIntoView().

As part of the resolution from
https://github.com/WICG/display-locking/issues/139

we decided that we should use the contained size information when
determining what to scroll into view. In the code, this means we
shouldn't activate the element on scrollIntoView and rely on viewport
intersection instead. Note that hidden-matchable now (correctly) doesn't
unhide.

R=chrishtr@chromium.org

Change-Id: I9336868ddace68b85e6da4b99ad02ad2f9aa1314
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109815Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752068}
parent 005d6c1f
rootWebArea
++genericContainer ignored
++++genericContainer ignored
++++genericContainer name='Done'
++++++genericContainer
++++++++genericContainer
++++++++++staticText name='child'
++++++++++++inlineTextBox name='child'
++++++++genericContainer
++++++++++staticText name='nested locked element!'
++++++++++++inlineTextBox name='nested locked element!'
++++++++genericContainer
<!--
@BLINK-ALLOW:offscreen
@WAIT-FOR:Done
-->
<div>
<div id="locked" style="subtree-visibility: hidden-matchable">
<div id=target aria-label="Working">
<div id="locked" style="subtree-visibility: auto">
<div>child</div>
<div id="nested" style="subtree-visibility: hidden-matchable">nested locked element!</div>
<div id="nested" style="subtree-visibility: auto">nested locked element!</div>
<div id="nonActivatable" style="subtree-visibility: hidden">nested non activatable locked element</div>
</div>
</div>
<script>
async function runTest() {
// Force layout, then activate.
locked.getBoundingClientRect();
locked.scrollIntoView();
// Double-rAF to ensure that both the outer and nested elements have enough
// time to process intersection observations.
requestAnimationFrame(
() => requestAnimationFrame(
() => target.setAttribute("aria-label", "Done")));
}
onload = () => requestAnimationFrame(runTest);
</script>
......@@ -363,7 +363,7 @@ void DisplayLockContext::FireActivationEvent(Element* activated_element) {
void DisplayLockContext::CommitForActivationWithSignal(
Element* activated_element,
DisplayLockActivationReason reason_for_metrics) {
DisplayLockActivationReason reason) {
DCHECK(activated_element);
DCHECK(element_);
DCHECK(ConnectedToView());
......@@ -377,8 +377,15 @@ void DisplayLockContext::CommitForActivationWithSignal(
weak_factory_.GetWeakPtr(), WrapPersistent(activated_element)));
}
RecordActivationReason(document_, reason);
// TODO(vmpstr): This should eventually only unlock on viewport intersection,
// but each reason needs to be tested and considered, so this is being done in
// parts.
if (reason == DisplayLockActivationReason::kScrollIntoView)
return;
Unlock();
RecordActivationReason(document_, reason_for_metrics);
is_activated_ = true;
}
......
......@@ -130,9 +130,8 @@ class CORE_EXPORT DisplayLockContext final
// This issues a before activate signal with the given element as the
// activated element.
// The reason is specified for metrics.
void CommitForActivationWithSignal(
Element* activated_element,
DisplayLockActivationReason reason_for_metrics);
void CommitForActivationWithSignal(Element* activated_element,
DisplayLockActivationReason reason);
bool ShouldCommitForActivation(DisplayLockActivationReason reason) const;
......
<!doctype HTML>
<!-- TODO(vmpstr): This test needs to change expectations once hidden-matchable
elements only send a signal and not activate
-->
<html class="reftest-wait">
<meta charset="utf8">
<title>subtree-visibility changes after a delay, activated by script</title>
<title>subtree-visibility changes after a delay</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="../container-with-child-ref.html">
<meta name="assert" content="scrollIntoView unhides the element">
<link rel="match" href="../container-ref.html">
<meta name="assert" content="scrollIntoView has no effect on hidden-matchable">
<script src="/common/reftest-wait.js"></script>
<style>
......
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>CSS Subtree Visibility: auto, scrollIntoView() (reference)</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://github.com/WICG/display-locking">
<script src="/common/reftest-wait.js"></script>
<style>
.spacer {
height: 10000px;
}
.container {
position: relative;
width: 150px;
background: lightblue;
contain-intrinsic-size: 50px 250px;
}
.size_contained {
contain: size;
}
.child {
width: 50px;
height: 300px;
background: lightgreen;
}
#target {
position: absolute;
bottom: 0;
width: 10px;
height: 10px;
background: blue;
}
</style>
<div class=spacer></div>
<div id=container class="container size_contained">
<div class=child></div>
<div id=target></div>
</div>
<div class=spacer></div>
<script>
function runReference() {
document.getElementById("target").scrollIntoView(true /* alignToTop */);
document.getElementById("container").classList.remove("size_contained");
requestAnimationFrame(takeScreenshot);
}
window.onload = requestAnimationFrame(() => {
requestAnimationFrame(() => {
requestAnimationFrame(() => {
runReference();
});
});
});
</script>
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>Subtree Visibility: auto, scrollIntoView()</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="subtree-visibility-063-ref.html">
<meta name="assert" content="scrollIntoView() uses contain:size information to find target offset">
<meta name="assert" content="viewport intersection removes contain:size thus moving target">
<script src="/common/reftest-wait.js"></script>
<style>
.spacer {
height: 10000px;
}
.container {
width: 150px;
background: lightblue;
contain-intrinsic-size: 50px 250px;
}
.child {
width: 50px;
height: 300px;
background: lightgreen;
}
#target {
position: absolute;
bottom: 0;
width: 10px;
height: 10px;
background: blue;
}
.auto { subtree-visibility: auto; }
</style>
<div class=spacer></div>
<div class="container auto">
<div class=child></div>
<div id=target></div>
</div>
<div class=spacer></div>
<script>
function runTest() {
document.getElementById("target").scrollIntoView(true /* alignToTop */);
requestAnimationFrame(takeScreenshot);
}
window.onload = requestAnimationFrame(() => {
requestAnimationFrame(() => {
requestAnimationFrame(() => {
runTest();
});
});
});
</script>
</html>
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