Skip to content
Permalink
Browse files
Scrolling with page granularity using keyboard smooth scrolling doesn…
…'t account for fixed content

https://bugs.webkit.org/show_bug.cgi?id=245746
rdar://100469555

Reviewed by Tim Horton.

When keyboard smooth scrolling is enabled, `KeyboardScrollingAnimator::scrollDistance`
is used, which is analagous to `ScrollableArea::scroll`. However, while the
latter accounts for page's having fixed content when scrolling with page
granularity, the former never does.

This PR fixes this by including the same logic that `ScrollableArea::scroll` has
to adjust the scroll step into `KeyboardScrollingAnimator::scrollDistance`.

* LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html:
* Source/WebCore/platform/KeyboardScrollingAnimator.cpp:
(WebCore::KeyboardScrollingAnimator::scrollDistance const):
* Source/WebCore/platform/ScrollableArea.h:

Canonical link: https://commits.webkit.org/254963@main
  • Loading branch information
rr-codes committed Sep 28, 2022
1 parent 219edaf commit fdd3720bd643eda26832e2155f1f939f08cd040a
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
@@ -27,6 +27,7 @@
#include "KeyboardScrollingAnimator.h"

#include "EventNames.h"
#include "FrameView.h"
#include "PlatformKeyboardEvent.h"
#include "ScrollTypes.h"
#include "ScrollableArea.h"
@@ -226,20 +227,30 @@ void KeyboardScrollingAnimator::updateKeyboardScrollPosition(MonotonicTime curre
float KeyboardScrollingAnimator::scrollDistance(ScrollDirection direction, ScrollGranularity granularity) const
{
auto scrollbar = m_scrollAnimator.scrollableArea().scrollbarForDirection(direction);
if (scrollbar) {
switch (granularity) {
case ScrollGranularity::Line:
return scrollbar->lineStep();
case ScrollGranularity::Page:
return scrollbar->pageStep();
case ScrollGranularity::Document:
return scrollbar->totalSize();
case ScrollGranularity::Pixel:
return scrollbar->pixelStep();
}
if (!scrollbar)
return false;

float step = 0;
switch (granularity) {
case ScrollGranularity::Line:
step = scrollbar->lineStep();
break;
case ScrollGranularity::Page:
step = scrollbar->pageStep();
break;
case ScrollGranularity::Document:
step = scrollbar->totalSize();
break;
case ScrollGranularity::Pixel:
step = scrollbar->pixelStep();
break;
}

return 0;
auto axis = axisFromDirection(direction);
if (granularity == ScrollGranularity::Page && axis == ScrollEventAxis::Vertical)
step = m_scrollAnimator.scrollableArea().adjustVerticalPageScrollStepForFixedContent(step);

return step;
}

std::optional<KeyboardScroll> KeyboardScrollingAnimator::makeKeyboardScroll(ScrollDirection direction, ScrollGranularity granularity) const
@@ -386,6 +386,7 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
bool overscrollBehaviorAllowsRubberBand() const { return horizontalOverscrollBehavior() != OverscrollBehavior::None || verticalOverscrollBehavior() != OverscrollBehavior::None; }
bool shouldBlockScrollPropagation(const FloatSize&) const;
FloatSize deltaForPropagation(const FloatSize&) const;
WEBCORE_EXPORT virtual float adjustVerticalPageScrollStepForFixedContent(float step);

protected:
WEBCORE_EXPORT ScrollableArea();
@@ -394,7 +395,6 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
void setScrollOrigin(const IntPoint&);
void resetScrollOriginChanged() { m_scrollOriginChanged = false; }

WEBCORE_EXPORT virtual float adjustVerticalPageScrollStepForFixedContent(float step);
virtual void invalidateScrollbarRect(Scrollbar&, const IntRect&) = 0;
virtual void invalidateScrollCornerRect(const IntRect&) = 0;

@@ -259,6 +259,7 @@ - (void)invalidate
}();

// FIXME (227461): Replace with call to WebCore::KeyboardScroll constructor.
// FIXME (245749): Use `ScrollableArea::adjustVerticalPageScrollStepForFixedContent` to account for fixed content

CGFloat scrollDistance = [_scrollable distanceForIncrement:increment inDirection:direction];

0 comments on commit fdd3720

Please sign in to comment.