зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1567237 - Only use scroll range to select scrollable frames to scroll to, don't use scrollbar visibility. r=tnikkel
This is what other browsers do, and it does make sense to me, it's useless to try to scroll a frame with no scroll range in a given direction. I think all callers of this function should be treated like this, so this is more like a RFC / feedback request than a patch per se. The wheel handling code already checks scroll range, so there's no difference of behavior in that case, if I'm reading the code right. There are a few other functions that check the result of GetPerceivedScrollingDirections(), but I think if we change this we should change this consistently. I also think that if we do this we should rename the method to something like GetAvailableScrollingDirections() or such. Anyhow, wdyt? I should also add a test for this if we go with this. Differential Revision: https://phabricator.services.mozilla.com/D38991 --HG-- extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez
Родитель
fffc7f0c58
Коммит
eb563e1090
|
|
@ -69,7 +69,7 @@ bool WheelHandlingUtils::CanScrollOn(nsIScrollableFrame* aScrollFrame,
|
|||
|
||||
nsPoint scrollPt = aScrollFrame->GetScrollPosition();
|
||||
nsRect scrollRange = aScrollFrame->GetScrollRange();
|
||||
uint32_t directions = aScrollFrame->GetPerceivedScrollingDirections();
|
||||
uint32_t directions = aScrollFrame->GetAvailableScrollingDirections();
|
||||
|
||||
return (aDirectionX && (directions & nsIScrollableFrame::HORIZONTAL) &&
|
||||
CanScrollInRange(scrollRange.x, scrollPt.x, scrollRange.XMost(),
|
||||
|
|
@ -741,12 +741,12 @@ void ESMAutoDirWheelDeltaAdjuster::OnAdjusted() {
|
|||
}
|
||||
|
||||
bool ESMAutoDirWheelDeltaAdjuster::CanScrollAlongXAxis() const {
|
||||
return mScrollTargetFrame->GetPerceivedScrollingDirections() &
|
||||
return mScrollTargetFrame->GetAvailableScrollingDirections() &
|
||||
nsIScrollableFrame::HORIZONTAL;
|
||||
}
|
||||
|
||||
bool ESMAutoDirWheelDeltaAdjuster::CanScrollAlongYAxis() const {
|
||||
return mScrollTargetFrame->GetPerceivedScrollingDirections() &
|
||||
return mScrollTargetFrame->GetAvailableScrollingDirections() &
|
||||
nsIScrollableFrame::VERTICAL;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3350,7 +3350,7 @@ static void ScrollToShowRect(nsIScrollableFrame* aFrameAsScrollable,
|
|||
ScrollStyles ss = aFrameAsScrollable->GetScrollStyles();
|
||||
nsRect allowedRange(scrollPt, nsSize(0, 0));
|
||||
bool needToScroll = false;
|
||||
uint32_t directions = aFrameAsScrollable->GetPerceivedScrollingDirections();
|
||||
uint32_t directions = aFrameAsScrollable->GetAvailableScrollingDirections();
|
||||
|
||||
if (((aScrollFlags & ScrollFlags::ScrollOverflowHidden) ||
|
||||
ss.mVertical != StyleOverflow::Hidden) &&
|
||||
|
|
|
|||
|
|
@ -2015,7 +2015,7 @@ nsIScrollableFrame* nsLayoutUtils::GetNearestScrollableFrameForDirection(
|
|||
nsIScrollableFrame* scrollableFrame = do_QueryFrame(f);
|
||||
if (scrollableFrame) {
|
||||
ScrollStyles ss = scrollableFrame->GetScrollStyles();
|
||||
uint32_t directions = scrollableFrame->GetPerceivedScrollingDirections();
|
||||
uint32_t directions = scrollableFrame->GetAvailableScrollingDirections();
|
||||
if (aDirection == eVertical
|
||||
? (ss.mVertical != StyleOverflow::Hidden &&
|
||||
(directions & nsIScrollableFrame::VERTICAL))
|
||||
|
|
|
|||
|
|
@ -419,6 +419,7 @@ support-files = resize_flush_iframe.html
|
|||
[test_scroll_event_ordering.html]
|
||||
[test_scroll_per_page.html]
|
||||
support-files = window_empty_document.html
|
||||
[test_scroll_space_no_range_overflow_scroll.html]
|
||||
[test_scroll_selection_into_view.html]
|
||||
skip-if = toolkit == 'android' # Bug 1355844
|
||||
support-files =
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
<!doctype html>
|
||||
<title>Test for bug 1567237</title>
|
||||
<script src="/resources/testharness.js"></script>
|
||||
<script src="/resources/testharnessreport.js"></script>
|
||||
<script src="/tests/SimpleTest/EventUtils.js"></script>
|
||||
<style>
|
||||
.spacer { height: 200vh; }
|
||||
.scroller { height: 300px; overflow: scroll; }
|
||||
</style>
|
||||
<div id="unscrollable" class="scroller" tabindex=0></div>
|
||||
<div id="scrollable" class="scroller" tabindex=0>
|
||||
<div class="spacer"></div>
|
||||
</div>
|
||||
<div class="spacer"></div>
|
||||
<script>
|
||||
function waitForScrollEvent(target) {
|
||||
return new Promise(resolve => {
|
||||
target.addEventListener("scroll", resolve, { once: true });
|
||||
});
|
||||
}
|
||||
|
||||
// Copy pasta from test_scroll_per_page, see there for the rationale.
|
||||
const IS_WIN = navigator.platform.includes("Win");
|
||||
const kUseKeyboardEvent = IS_WIN;
|
||||
let selectionController;
|
||||
if (!kUseKeyboardEvent) {
|
||||
selectionController = SpecialPowers.wrap(window)
|
||||
.docShell
|
||||
.QueryInterface(SpecialPowers.Ci.nsIInterfaceRequestor)
|
||||
.getInterface(SpecialPowers.Ci.nsISelectionDisplay)
|
||||
.QueryInterface(SpecialPowers.Ci.nsISelectionController);
|
||||
}
|
||||
|
||||
function doPageDown(targetExpectedToScroll) {
|
||||
let promise = waitForScrollEvent(targetExpectedToScroll);
|
||||
if (kUseKeyboardEvent) {
|
||||
synthesizeKey("KEY_PageDown", {}, aWindow);
|
||||
} else {
|
||||
selectionController.pageMove(true, false);
|
||||
}
|
||||
return promise;
|
||||
}
|
||||
|
||||
promise_test(async function() {
|
||||
await SpecialPowers.pushPrefEnv({"set": [["general.smoothScroll", false]]});
|
||||
|
||||
const rootScroller = document.documentElement;
|
||||
const scrollable = document.querySelector("#scrollable");
|
||||
const unscrollable = document.querySelector("#unscrollable");
|
||||
|
||||
assert_equals(rootScroller.scrollTop, 0, "Root should start unscrolled");
|
||||
assert_equals(scrollable.scrollTop, 0, "#scrollable should start unscrolled");
|
||||
assert_equals(unscrollable.scrollTop, 0, "#unscrollable should not be able to scroll at all");
|
||||
|
||||
assert_true(rootScroller.scrollTopMax > 0, "Should be able to scroll the document element");
|
||||
assert_true(scrollable.scrollTopMax > 0, "Should be able to scroll #scrollable");
|
||||
assert_equals(unscrollable.scrollTopMax, 0, "#unscrollable should not be able to scroll at all (checking scrollTopMax)");
|
||||
|
||||
{
|
||||
scrollable.focus();
|
||||
await doPageDown(scrollable);
|
||||
assert_not_equals(scrollable.scrollTop, 0, "Should have scrolled when pressing space");
|
||||
}
|
||||
{
|
||||
unscrollable.focus();
|
||||
let rootScrollTop = rootScroller.scrollTop; // Could've scrolled to scroll `scrollable` into view before.
|
||||
await doPageDown(window);
|
||||
assert_equals(unscrollable.scrollTop, 0, "Should not be able to scroll the unscrollable div");
|
||||
assert_not_equals(rootScroller.scrollTop, rootScrollTop, "Root should be able to scroll");
|
||||
}
|
||||
}, "Overflow scroll without range doesn't block scrolling of the main document");
|
||||
</script>
|
||||
|
|
@ -1622,7 +1622,7 @@ nsIFrame* nsFrameSelection::GetFrameToPageSelect() const {
|
|||
if (scrollStyles.mVertical == StyleOverflow::Hidden) {
|
||||
continue;
|
||||
}
|
||||
uint32_t directions = scrollableFrame->GetPerceivedScrollingDirections();
|
||||
uint32_t directions = scrollableFrame->GetAvailableScrollingDirections();
|
||||
if (directions & nsIScrollableFrame::VERTICAL) {
|
||||
// If there is sub scrollable frame, let's use its page size to select.
|
||||
return frame;
|
||||
|
|
|
|||
|
|
@ -6654,10 +6654,10 @@ void ScrollFrameHelper::FireScrolledAreaEvent() {
|
|||
}
|
||||
}
|
||||
|
||||
uint32_t nsIScrollableFrame::GetPerceivedScrollingDirections() const {
|
||||
uint32_t nsIScrollableFrame::GetAvailableScrollingDirections() const {
|
||||
nscoord oneDevPixel =
|
||||
GetScrolledFrame()->PresContext()->AppUnitsPerDevPixel();
|
||||
uint32_t directions = GetScrollbarVisibility();
|
||||
uint32_t directions = 0;
|
||||
nsRect scrollRange = GetScrollRange();
|
||||
if (scrollRange.width >= oneDevPixel) {
|
||||
directions |= HORIZONTAL;
|
||||
|
|
|
|||
|
|
@ -81,11 +81,10 @@ class nsIScrollableFrame : public nsIScrollbarMediator {
|
|||
*/
|
||||
virtual uint32_t GetScrollbarVisibility() const = 0;
|
||||
/**
|
||||
* Returns the directions in which scrolling is perceived to be allowed.
|
||||
* A direction is perceived to be allowed if there is a visible scrollbar
|
||||
* for that direction or if the scroll range is at least one device pixel.
|
||||
* Returns the directions in which scrolling is allowed (if the scroll range
|
||||
* is at least one device pixel in that direction).
|
||||
*/
|
||||
uint32_t GetPerceivedScrollingDirections() const;
|
||||
uint32_t GetAvailableScrollingDirections() const;
|
||||
/**
|
||||
* Return the actual sizes of all possible scrollbars. Returns 0 for scrollbar
|
||||
* positions that don't have a scrollbar or where the scrollbar is not
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче