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

SubtreeVisibility: Allow scroll to text activations to span multiple blocks.

This patch makes it so that the activation due to scroll to text is
allowed to span multiple blocks. The activated element is the block at
the beginning of the range.

R=chrishtr@chromium.org

Bug: https://github.com/WICG/display-locking/issues/125
Change-Id: I4223a59f8910175aaa5c1e877afd2d1e70f9ec93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083574Reviewed-by: default avatarJoey Arhar <jarhar@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746080}
parent 20c71a0d
......@@ -75,9 +75,13 @@ bool DisplayLockUtilities::ActivateFindInPageMatchRangeIfNeeded(
// case we are traversing from the start position of the range.
Element* enclosing_block =
EnclosingBlock(range.StartPosition(), kCannotCrossEditingBoundary);
// Note that we don't check the `range.EndPosition()` since we just activate
// the beginning of the range. In find-in-page cases, the end position is the
// same since the matches cannot cross block boundaries. However, in
// scroll-to-text, the range might be different, but we still just activate
// the beginning of the range. See
// https://github.com/WICG/display-locking/issues/125 for more details.
DCHECK(enclosing_block);
DCHECK_EQ(enclosing_block,
EnclosingBlock(range.EndPosition(), kCannotCrossEditingBoundary));
return enclosing_block->ActivateDisplayLockIfNeeded(
DisplayLockActivationReason::kFindInPage);
}
......
......@@ -14,6 +14,10 @@ function checkScroll() {
position = 'top';
else if(isInView(document.getElementById("text")))
position = 'text';
else if(isInView(document.getElementById("text2")))
position = 'text2';
else if(isInView(document.getElementById("text3")))
position = 'text3';
const target = document.querySelector(":target");
let results = {
......@@ -41,4 +45,14 @@ function checkScroll() {
<div id=text>lockedtext</div>
</div>
<div class=spacer></div>
<div id=text2and3ancestor>
<div class=locked>
<div id=text2>start</div>
</div>
<div class=spacer></div>
<div class=locked>
<div id=text3>end</div>
</div>
<div class=spacer></div>
</div>
</body>
......@@ -11,8 +11,8 @@
<script src="text-fragment-helper.js"></script>
<script>
const fragment = '#:~:text=lockedtext';
promise_test(t => new Promise((resolve, reject) => {
const fragment = '#:~:text=lockedtext';
const key = token();
test_driver.bless("Open a URL with a text fragment directive", () => {
window.open(`text-fragment-navigation-activates-target.html?key=${key}${fragment}`,
......@@ -21,8 +21,22 @@ promise_test(t => new Promise((resolve, reject) => {
});
fetchResults(key, resolve, reject);
}).then(data => {
assert_equals(data.scrollPosition, "text");
assert_equals(data.target, "text");
assert_equals(data.scrollPosition, "text", "scroll position");
assert_equals(data.target, "text", "target");
}), "Fragment navigation with render-subtree");
promise_test(t => new Promise((resolve, reject) => {
const fragment = '#:~:text=start,end';
const key = token();
test_driver.bless("Open a URL with a text fragment directive", () => {
window.open(`text-fragment-navigation-activates-target.html?key=${key}${fragment}`,
'_blank',
'noopener');
});
fetchResults(key, resolve, reject);
}).then(data => {
assert_equals(data.scrollPosition, "text2", "scroll position");
assert_equals(data.target, "text2and3ancestor", "target");
}), "Fragment navigation with render-subtree spans two locks");
</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