Diff
Modified: trunk/Source/WebCore/ChangeLog (286118 => 286119)
--- trunk/Source/WebCore/ChangeLog 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/ChangeLog 2021-11-22 21:30:08 UTC (rev 286119)
@@ -1,3 +1,30 @@
+2021-11-22 Simon Fraser <[email protected]>
+
+ Clarify the behavior of ScrollAnimator::scroll()
+ https://bugs.webkit.org/show_bug.cgi?id=233403
+
+ Unreviewed.
+
+ Address patch feedback from Chris Lord.
+
+ * dom/Element.cpp:
+ (WebCore::Element::scrollByUnits):
+ * page/FrameView.cpp:
+ (WebCore::FrameView::adjustVerticalPageScrollStepForFixedContent):
+ (WebCore::FrameView::adjustScrollStepForFixedContent): Deleted.
+ * page/FrameView.h:
+ * platform/ScrollAnimator.cpp:
+ (WebCore::ScrollAnimator::singleAxisScroll):
+ * platform/ScrollTypes.h:
+ (WebCore::axisFromDirection):
+ (WebCore::valueForAxis):
+ (WebCore::setValueForAxis):
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::adjustVerticalPageScrollStepForFixedContent):
+ (WebCore::ScrollableArea::scroll):
+ (WebCore::ScrollableArea::adjustScrollStepForFixedContent): Deleted.
+ * platform/ScrollableArea.h:
+
2021-11-22 Fujii Hironori <[email protected]>
[MSVC] RenderBlock.cpp(2259): warning C4239: nonstandard extension used: 'initializing': conversion from 'WebCore::Length' to 'WebCore::Length &'
Modified: trunk/Source/WebCore/dom/Element.cpp (286118 => 286119)
--- trunk/Source/WebCore/dom/Element.cpp 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/dom/Element.cpp 2021-11-22 21:30:08 UTC (rev 286119)
@@ -1118,10 +1118,7 @@
if (!renderer->hasNonVisibleOverflow())
return;
- ScrollDirection direction = ScrollDown;
- if (units < 0)
- direction = ScrollUp;
-
+ auto direction = units < 0 ? ScrollUp : ScrollDown;
auto* stopElement = this;
downcast<RenderBox>(*renderer).scroll(direction, granularity, std::abs(units), &stopElement);
}
Modified: trunk/Source/WebCore/page/FrameView.cpp (286118 => 286119)
--- trunk/Source/WebCore/page/FrameView.cpp 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/page/FrameView.cpp 2021-11-22 21:30:08 UTC (rev 286119)
@@ -3834,11 +3834,8 @@
setCurrentScrollType(previousScrollType);
}
-float FrameView::adjustScrollStepForFixedContent(float step, ScrollEventAxis axis, ScrollGranularity granularity)
+float FrameView::adjustVerticalPageScrollStepForFixedContent(float step)
{
- if (granularity != ScrollGranularity::Page || axis == ScrollEventAxis::Horizontal)
- return step;
-
TrackedRendererListHashSet* positionedObjects = nullptr;
if (RenderView* root = frame().contentRenderer()) {
if (!root->hasPositionedObjects())
Modified: trunk/Source/WebCore/page/FrameView.h (286118 => 286119)
--- trunk/Source/WebCore/page/FrameView.h 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/page/FrameView.h 2021-11-22 21:30:08 UTC (rev 286119)
@@ -661,7 +661,7 @@
bool isScrollSnapInProgress() const final;
void updateScrollingCoordinatorScrollSnapProperties() const;
- float adjustScrollStepForFixedContent(float step, ScrollEventAxis, ScrollGranularity) final;
+ float adjustVerticalPageScrollStepForFixedContent(float step) final;
void didChangeScrollOffset();
Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (286118 => 286119)
--- trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-11-22 21:30:08 UTC (rev 286119)
@@ -69,16 +69,13 @@
auto delta = setValueForAxis(FloatSize { }, axis, scrollDelta);
- if (behavior.contains(ScrollBehavior::RespectScrollSnap)) {
- behavior.remove(ScrollBehavior::RespectScrollSnap);
- if (m_scrollController.usesScrollSnap()) {
- auto currentOffset = offsetFromPosition(currentPosition());
- auto newOffset = currentOffset + delta;
- auto velocity = copysignf(1.0f, scrollDelta);
- auto newOffsetOnAxis = m_scrollController.adjustedScrollDestination(axis, newOffset, velocity, valueForAxis(currentOffset, axis));
- newOffset = setValueForAxis(newOffset, axis, newOffsetOnAxis);
- delta = newOffset - currentOffset;
- }
+ if (behavior.contains(ScrollBehavior::RespectScrollSnap) && m_scrollController.usesScrollSnap()) {
+ auto currentOffset = offsetFromPosition(currentPosition());
+ auto newOffset = currentOffset + delta;
+ auto velocity = copysignf(1.0f, scrollDelta);
+ auto newOffsetOnAxis = m_scrollController.adjustedScrollDestination(axis, newOffset, velocity, valueForAxis(currentOffset, axis));
+ newOffset = setValueForAxis(newOffset, axis, newOffsetOnAxis);
+ delta = newOffset - currentOffset;
}
if (m_scrollableArea.scrollAnimatorEnabled() && platformAllowsScrollAnimation() && !behavior.contains(ScrollBehavior::NeverAnimate)) {
Modified: trunk/Source/WebCore/platform/ScrollTypes.h (286118 => 286119)
--- trunk/Source/WebCore/platform/ScrollTypes.h 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/platform/ScrollTypes.h 2021-11-22 21:30:08 UTC (rev 286119)
@@ -179,6 +179,7 @@
case ScrollLeft: return ScrollEventAxis::Horizontal;
case ScrollRight: return ScrollEventAxis::Horizontal;
}
+ ASSERT_NOT_REACHED();
return ScrollEventAxis::Vertical;
}
@@ -188,6 +189,7 @@
case ScrollEventAxis::Horizontal: return size.width();
case ScrollEventAxis::Vertical: return size.height();
}
+ ASSERT_NOT_REACHED();
return 0;
}
@@ -201,6 +203,7 @@
size.setHeight(value);
return size;
}
+ ASSERT_NOT_REACHED();
return size;
}
@@ -210,6 +213,7 @@
case ScrollEventAxis::Horizontal: return point.x();
case ScrollEventAxis::Vertical: return point.y();
}
+ ASSERT_NOT_REACHED();
return 0;
}
@@ -222,6 +226,7 @@
case ScrollEventAxis::Vertical: point.setY(value);
return point;
}
+ ASSERT_NOT_REACHED();
return point;
}
Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (286118 => 286119)
--- trunk/Source/WebCore/platform/ScrollableArea.cpp 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp 2021-11-22 21:30:08 UTC (rev 286119)
@@ -91,7 +91,7 @@
}
}
-float ScrollableArea::adjustScrollStepForFixedContent(float step, ScrollEventAxis, ScrollGranularity)
+float ScrollableArea::adjustVerticalPageScrollStepForFixedContent(float step)
{
return step;
}
@@ -119,7 +119,10 @@
}
auto axis = axisFromDirection(direction);
- step = adjustScrollStepForFixedContent(step, axis, granularity);
+
+ if (granularity == ScrollGranularity::Page && axis == ScrollEventAxis::Vertical)
+ step = adjustVerticalPageScrollStepForFixedContent(step);
+
auto scrollDelta = step * stepCount;
if (direction == ScrollUp || direction == ScrollLeft)
Modified: trunk/Source/WebCore/platform/ScrollableArea.h (286118 => 286119)
--- trunk/Source/WebCore/platform/ScrollableArea.h 2021-11-22 20:42:11 UTC (rev 286118)
+++ trunk/Source/WebCore/platform/ScrollableArea.h 2021-11-22 21:30:08 UTC (rev 286119)
@@ -378,7 +378,7 @@
void setScrollOrigin(const IntPoint&);
void resetScrollOriginChanged() { m_scrollOriginChanged = false; }
- WEBCORE_EXPORT virtual float adjustScrollStepForFixedContent(float step, ScrollEventAxis, ScrollGranularity);
+ WEBCORE_EXPORT virtual float adjustVerticalPageScrollStepForFixedContent(float step);
virtual void invalidateScrollbarRect(Scrollbar&, const IntRect&) = 0;
virtual void invalidateScrollCornerRect(const IntRect&) = 0;
