Bug 1701928 - Selection scrolling should not ignore scroll-{margin,padding}. r=hiro,masayuki

Differential Revision: https://phabricator.services.mozilla.com/D110440
This commit is contained in:
Emilio Cobos Álvarez
2021-04-02 12:14:19 +00:00
parent d7b5fd7086
commit 79cfc09871
4 changed files with 54 additions and 13 deletions

View File

@@ -1810,7 +1810,7 @@ nsresult AutoScroller::DoAutoScroll(nsIFrame* aFrame, nsPoint aPoint) {
while (true) { while (true) {
didScroll = presShell->ScrollFrameRectIntoView( didScroll = presShell->ScrollFrameRectIntoView(
aFrame, nsRect(aPoint, nsSize(0, 0)), ScrollAxis(), ScrollAxis(), aFrame, nsRect(aPoint, nsSize(0, 0)), ScrollAxis(), ScrollAxis(),
ScrollFlags::IgnoreMarginAndPadding); ScrollFlags::None);
if (!weakFrame || !weakRootFrame) { if (!weakFrame || !weakRootFrame) {
return NS_OK; return NS_OK;
} }
@@ -2996,7 +2996,7 @@ nsresult Selection::ScrollIntoView(SelectionRegion aRegion,
// vertical scrollbar or the scroll range is at least one device pixel) // vertical scrollbar or the scroll range is at least one device pixel)
aVertical.mOnlyIfPerceivedScrollableDirection = true; aVertical.mOnlyIfPerceivedScrollableDirection = true;
ScrollFlags scrollFlags = ScrollFlags::IgnoreMarginAndPadding; auto scrollFlags = ScrollFlags::None;
if (aFlags & Selection::SCROLL_FIRST_ANCESTOR_ONLY) { if (aFlags & Selection::SCROLL_FIRST_ANCESTOR_ONLY) {
scrollFlags |= ScrollFlags::ScrollFirstAncestorOnly; scrollFlags |= ScrollFlags::ScrollFirstAncestorOnly;
} }

View File

@@ -3414,10 +3414,7 @@ static void ScrollToShowRect(nsIScrollableFrame* aFrameAsScrollable,
const nsRect visibleRect(scrollPt, const nsRect visibleRect(scrollPt,
aFrameAsScrollable->GetVisualViewportSize()); aFrameAsScrollable->GetVisualViewportSize());
const nsMargin scrollPadding = const nsMargin scrollPadding = aFrameAsScrollable->GetScrollPadding();
(aScrollFlags & ScrollFlags::IgnoreMarginAndPadding)
? nsMargin()
: aFrameAsScrollable->GetScrollPadding();
const nsRect rectToScrollIntoView = [&] { const nsRect rectToScrollIntoView = [&] {
nsRect r(aRect); nsRect r(aRect);
@@ -3586,10 +3583,7 @@ void PresShell::DoScrollContentIntoView() {
// Get the scroll-margin here since |frame| is going to be changed to iterate // Get the scroll-margin here since |frame| is going to be changed to iterate
// over all continuation frames below. // over all continuation frames below.
nsMargin scrollMargin; const nsMargin scrollMargin = frame->StyleMargin()->GetScrollMargin();
if (!(data->mContentToScrollToFlags & ScrollFlags::IgnoreMarginAndPadding)) {
scrollMargin = frame->StyleMargin()->GetScrollMargin();
}
// This is a two-step process. // This is a two-step process.
// Step 1: Find the bounds of the rect we want to scroll into view. For // Step 1: Find the bounds of the rect we want to scroll into view. For
@@ -8974,8 +8968,7 @@ bool PresShell::EventHandler::PrepareToUseCaretPosition(
->ScrollContentIntoView( ->ScrollContentIntoView(
content, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible), content, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible), ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollFlags::ScrollOverflowHidden | ScrollFlags::ScrollOverflowHidden);
ScrollFlags::IgnoreMarginAndPadding);
NS_ENSURE_SUCCESS(rv, false); NS_ENSURE_SUCCESS(rv, false);
frame = content->GetPrimaryFrame(); frame = content->GetPrimaryFrame();
NS_WARNING_ASSERTION(frame, "No frame for focused content?"); NS_WARNING_ASSERTION(frame, "No frame for focused content?");

View File

@@ -137,7 +137,6 @@ enum class ScrollFlags {
ScrollSmooth = 1 << 3, ScrollSmooth = 1 << 3,
ScrollSmoothAuto = 1 << 4, ScrollSmoothAuto = 1 << 4,
ScrollSnap = 1 << 5, ScrollSnap = 1 << 5,
IgnoreMarginAndPadding = 1 << 6,
// ScrollOverflowHidden | ScrollNoParentFrames // ScrollOverflowHidden | ScrollNoParentFrames
AnchorScrollFlags = (1 << 1) | (1 << 2), AnchorScrollFlags = (1 << 1) | (1 << 2),
ALL_BITS = (1 << 7) - 1, ALL_BITS = (1 << 7) - 1,

View File

@@ -0,0 +1,49 @@
<!doctype html>
<meta charset=utf-8>
<title>scroll-padding is respected when typing into an out-of-view textfield</title>
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1701928">
<link rel="help" href="https://drafts.csswg.org/css-scroll-snap/#scroll-padding">
<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
<link rel="author" href="https://mozilla.org" title="Mozilla">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<style>
body {
margin: 0;
}
:root {
scroll-padding-top: 100px;
}
</style>
<div style="height: 300vh;"></div>
<input type="text">
<div style="height: 300vh;"></div>
<script>
function tick() {
return new Promise(resolve => {
requestAnimationFrame(() => requestAnimationFrame(resolve));
});
}
promise_test(async function() {
let input = document.querySelector("input");
input.focus();
await tick();
// Scroll out of view.
scrollTo(0, document.scrollingElement.scrollHeight);
await tick();
assert_not_equals(window.scrollY, 0);
assert_true(input.getBoundingClientRect().bottom < 0, "Should be offscreen");
// NOTE(emilio): Using test_driver.Actions() instead of test_driver.send_keys
// because the later scrolls the target into view which would defeat the
// point of the test...
await new test_driver.Actions().keyDown('a').send();
await tick();
// We assert the bottom rather than the top because Gecko scrolls the
// selection bounds into view, not the whole input.
assert_true(input.getBoundingClientRect().bottom > 100, "Scroll-padding should be respected");
}, "Test scrolling into view when typing");
</script>