commit a99f99fe918131e0acf6cf7c1f83ebe20828572a
parent c6b00049a9371cfb2781e9c111772f6eb20dc492
Author: Hiroyuki Ikezoe <hikezoe.birchill@mozilla.com>
Date: Fri, 12 Dec 2025 03:09:17 +0000
Bug 2002407 - Drop mOnlyIfPerceivedScrollableDirection. r=layout-reviewers,dholbert
mOnlyIfPerceivedScrollableDirection flag was introduced in bug 791616 to fix
an issue that while moving the caret the box having the caret was
slightly scrolled. There's a mochitest named test_bug791616.html [1] in
our tree for bug 791616, now the mochitest gets passed without the flag.
So it isn't necessary now.
helper_scrollIntoView_bug2002407 is a test case for bug 1999977 which
should be fixed by this change.
[1] https://searchfox.org/firefox-main/rev/7032b912c282da6b9b8219e779b2a441a27aaec9/layout/generic/test/test_bug791616.html
Differential Revision: https://phabricator.services.mozilla.com/D275903
Diffstat:
6 files changed, 93 insertions(+), 36 deletions(-)
diff --git a/dom/base/Selection.cpp b/dom/base/Selection.cpp
@@ -3880,10 +3880,6 @@ nsresult Selection::ScrollIntoView(SelectionRegion aRegion,
return NS_ERROR_FAILURE;
}
- // Scroll vertically to get the caret into view, but only if the container
- // is perceived to be scrollable in that direction (i.e. there is a visible
- // vertical scrollbar or the scroll range is at least one device pixel)
- aVertical.mOnlyIfPerceivedScrollableDirection = true;
presShell->ScrollFrameIntoView(frame, Some(rect), aVertical, aHorizontal,
aScrollFlags);
return NS_OK;
diff --git a/dom/ipc/TabMessageUtils.h b/dom/ipc/TabMessageUtils.h
@@ -79,7 +79,6 @@ struct ParamTraits<mozilla::ScrollAxis> {
static void Write(MessageWriter* aWriter, const paramType& aParam) {
WriteParam(aWriter, aParam.mWhereToScroll);
WriteParam(aWriter, aParam.mWhenToScroll);
- WriteParam(aWriter, aParam.mOnlyIfPerceivedScrollableDirection);
}
static bool Read(MessageReader* aReader, paramType* aResult) {
@@ -89,15 +88,6 @@ struct ParamTraits<mozilla::ScrollAxis> {
if (!ReadParam(aReader, &aResult->mWhenToScroll)) {
return false;
}
-
- // We can't set mOnlyIfPerceivedScrollableDirection directly since it's
- // a bitfield.
- bool value;
- if (!ReadParam(aReader, &value)) {
- return false;
- }
- aResult->mOnlyIfPerceivedScrollableDirection = value;
-
return true;
}
};
diff --git a/gfx/layers/apz/test/mochitest/helper_scrollIntoView_bug2002407.html b/gfx/layers/apz/test/mochitest/helper_scrollIntoView_bug2002407.html
@@ -0,0 +1,83 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width,initial-scale=1">
+ <title>Tests that moving the caret scrolls the content editable box even if the box is not scrollable</title>
+ <script src="apz_test_utils.js"></script>
+ <script src="/tests/SimpleTest/EventUtils.js"></script>
+ <script src="/tests/SimpleTest/paint_listener.js"></script>
+</head>
+<style>
+@font-face {
+ font-family: Ahem;
+ src: url("/tests/layout/base/tests/Ahem.ttf");
+}
+
+body {
+ margin: 0px;
+ padding: 0px;
+}
+#target {
+ width: 100%;
+ font: 14px/1 Ahem;
+}
+</style>
+<body>
+<div style="height: 80vh;"></div>
+<div id="target" contenteditable>1234567890</div>
+<script>
+async function test() {
+ const utils = SpecialPowers.getDOMWindowUtils(window);
+
+ is(window.scrollY, 0, "The initial scroll offset should be 0");
+ is(visualViewport.scale, 2.0, "The document should get scaled by 2.0");
+ is(visualViewport.pageTop, 0, "The initial visual viewport pageTop should be 0");
+
+ const target = document.querySelector("#target");
+
+ // Focus to the contenteditable element without scrolling.
+ const focusPromise = new Promise(resolve => {
+ target.addEventListener("focus", resolve, { once: true });
+ });
+ target.focus({ preventScroll: true });
+ await focusPromise;
+
+ let visualScrollPromise = new Promise(resolve => {
+ visualViewport.addEventListener("scroll", resolve, { once: true });
+ });
+ target.scrollIntoView({ block: "nearest" });
+ await visualScrollPromise;
+ ok(visualViewport.offsetTop > 0,
+ `visualViewport.offsetTop: ${visualViewport.offsetTop} should be greater than 0`);
+ const offsetTop = visualViewport.offsetTop;
+
+ // Visualy scroll back to (0, 0).
+ // Now the contentediable element is out of view.
+ utils.scrollToVisual(0, 0,
+ utils.UPDATE_TYPE_MAIN_THREAD,
+ utils.SCROLL_MODE_INSTANT);
+ await promiseApzFlushedRepaints();
+ is(visualViewport.offsetTop, 0);
+
+ visualScrollPromise = new Promise(resolve => {
+ visualViewport.addEventListener("scroll", resolve, { once: true });
+ });
+ const selection = window.getSelection();
+ selection.collapse(target.firstChild, 1);
+ // Now move the caret and then see whether the contenteditable element
+ // scrolled into the visual viewport.
+ synthesizeKey("KEY_ArrowLeft");
+ await visualScrollPromise;
+
+ isfuzzy(visualViewport.offsetTop, offsetTop, 1.0,
+ "The content editable box should be inside the visual viewport");
+}
+
+SpecialPowers.getDOMWindowUtils(window).setResolutionAndScaleTo(2.0);
+waitUntilApzStable()
+.then(test)
+.then(subtestDone, subtestFailed);
+</script>
+</body>
+</html>
diff --git a/gfx/layers/apz/test/mochitest/test_group_scrollIntoView.html b/gfx/layers/apz/test/mochitest/test_group_scrollIntoView.html
@@ -35,6 +35,10 @@ var subtests = [
"prefs": [...prefs,
["layout.scroll_fixed_content_into_view_visually", true]]
},
+ {"file": "helper_scrollIntoView_bug2002407.html",
+ "prefs": [...prefs,
+ ["layout.scroll_fixed_content_into_view_visually", true]]
+ },
];
if (isApzEnabled()) {
diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp
@@ -3512,13 +3512,9 @@ static Maybe<nsPoint> ScrollToShowRect(
}
ScrollStyles ss = aScrollContainerFrame->GetScrollStyles();
nsRect allowedRange(scrollPt, nsSize(0, 0));
- ScrollDirections directions =
- aScrollContainerFrame->GetAvailableScrollingDirections();
- if (((aScrollFlags & ScrollFlags::ScrollOverflowHidden) ||
- ss.mVertical != StyleOverflow::Hidden) &&
- (!aVertical.mOnlyIfPerceivedScrollableDirection ||
- (directions.contains(ScrollDirection::eVertical)))) {
+ if ((aScrollFlags & ScrollFlags::ScrollOverflowHidden) ||
+ ss.mVertical != StyleOverflow::Hidden) {
if (ComputeNeedToScroll(aVertical.mWhenToScroll, lineSize.height, aRect.y,
aRect.YMost(), visibleRect.y + padding.top,
visibleRect.YMost() - padding.bottom)) {
@@ -3536,10 +3532,8 @@ static Maybe<nsPoint> ScrollToShowRect(
}
}
- if (((aScrollFlags & ScrollFlags::ScrollOverflowHidden) ||
- ss.mHorizontal != StyleOverflow::Hidden) &&
- (!aHorizontal.mOnlyIfPerceivedScrollableDirection ||
- (directions.contains(ScrollDirection::eHorizontal)))) {
+ if ((aScrollFlags & ScrollFlags::ScrollOverflowHidden) ||
+ ss.mHorizontal != StyleOverflow::Hidden) {
if (ComputeNeedToScroll(aHorizontal.mWhenToScroll, lineSize.width, aRect.x,
aRect.XMost(), visibleRect.x + padding.left,
visibleRect.XMost() - padding.right)) {
diff --git a/layout/base/PresShellForwards.h b/layout/base/PresShellForwards.h
@@ -119,23 +119,13 @@ struct ScrollAxis final {
* visible.
* * WhenToScroll::Always: Move the frame regardless of its current
* visibility.
- *
- * aOnlyIfPerceivedScrollableDirection:
- * If the direction is not a perceived scrollable direction (i.e. no
- * scrollbar showing and less than one device pixel of scrollable
- * distance), don't scroll. Defaults to false.
*/
explicit ScrollAxis(WhereToScroll aWhere = WhereToScroll::Nearest,
- WhenToScroll aWhen = WhenToScroll::IfNotFullyVisible,
- bool aOnlyIfPerceivedScrollableDirection = false)
- : mWhereToScroll(aWhere),
- mWhenToScroll(aWhen),
- mOnlyIfPerceivedScrollableDirection(
- aOnlyIfPerceivedScrollableDirection) {}
+ WhenToScroll aWhen = WhenToScroll::IfNotFullyVisible)
+ : mWhereToScroll(aWhere), mWhenToScroll(aWhen) {}
WhereToScroll mWhereToScroll;
WhenToScroll mWhenToScroll;
- bool mOnlyIfPerceivedScrollableDirection : 1;
};
enum class ScrollFlags : uint8_t {
