- Revision
- 234596
- Author
- commit-qu...@webkit.org
- Date
- 2018-08-06 02:56:08 -0700 (Mon, 06 Aug 2018)
Log Message
Make two-arguments versions of scrollBy/scrollTo depend on the one-argument versions
https://bugs.webkit.org/show_bug.cgi?id=188300
Patch by Frederic Wang <fw...@igalia.com> on 2018-08-06
Reviewed by Darin Adler.
This patch refactors a bit the scrollBy/scrollTo code, so that the two-arguments versions
share the same code path as the more generic one-argument versions. In particular, this
helps to implement the ScrollBehavior option (bug 188043) since the one-argument versions
will require to distinguish between smooth and instant scrolling. The logic to normalize
non finite left/right values or to use a fallback when they are absent is also factored out
into ScrollToOptions.
References:
https://drafts.csswg.org/cssom-view/#dom-element-scroll
https://drafts.csswg.org/cssom-view/#dom-element-scrollby
https://drafts.csswg.org/cssom-view/#dom-window-scroll
https://drafts.csswg.org/cssom-view/#dom-window-scrollby
No new tests, behavior is unchanged.
* dom/Element.cpp:
(WebCore::Element::scrollBy): Make two-parameter version depends on one-parameter version
and rewrite the normalize / fallback logic.
(WebCore::Element::scrollTo): Rewrite the normalize / fallback logic.
(WebCore::normalizeNonFiniteValue): Deleted. The logic is moved to ScrollToOptions.
* page/DOMWindow.cpp:
(WebCore::DOMWindow::scrollBy const): Make two-parameter version depends on one-parameter
version and rewrite the normalize / fallback logic.
(WebCore::DOMWindow::scrollTo const): Make two-parameter version depends on one-parameter
version and rewrite the normalize / fallback logic.
* page/ScrollToOptions.h: Add <cmath> to use std::isfinite
(WebCore::ScrollToOptions::normalizeNonFiniteCoordinatesOrFallBackTo): New function to
normalize left/right values or fallback to the specified value if it is missing.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (234595 => 234596)
--- trunk/Source/WebCore/ChangeLog 2018-08-06 09:24:36 UTC (rev 234595)
+++ trunk/Source/WebCore/ChangeLog 2018-08-06 09:56:08 UTC (rev 234596)
@@ -1,3 +1,39 @@
+2018-08-06 Frederic Wang <fw...@igalia.com>
+
+ Make two-arguments versions of scrollBy/scrollTo depend on the one-argument versions
+ https://bugs.webkit.org/show_bug.cgi?id=188300
+
+ Reviewed by Darin Adler.
+
+ This patch refactors a bit the scrollBy/scrollTo code, so that the two-arguments versions
+ share the same code path as the more generic one-argument versions. In particular, this
+ helps to implement the ScrollBehavior option (bug 188043) since the one-argument versions
+ will require to distinguish between smooth and instant scrolling. The logic to normalize
+ non finite left/right values or to use a fallback when they are absent is also factored out
+ into ScrollToOptions.
+
+ References:
+ https://drafts.csswg.org/cssom-view/#dom-element-scroll
+ https://drafts.csswg.org/cssom-view/#dom-element-scrollby
+ https://drafts.csswg.org/cssom-view/#dom-window-scroll
+ https://drafts.csswg.org/cssom-view/#dom-window-scrollby
+
+ No new tests, behavior is unchanged.
+
+ * dom/Element.cpp:
+ (WebCore::Element::scrollBy): Make two-parameter version depends on one-parameter version
+ and rewrite the normalize / fallback logic.
+ (WebCore::Element::scrollTo): Rewrite the normalize / fallback logic.
+ (WebCore::normalizeNonFiniteValue): Deleted. The logic is moved to ScrollToOptions.
+ * page/DOMWindow.cpp:
+ (WebCore::DOMWindow::scrollBy const): Make two-parameter version depends on one-parameter
+ version and rewrite the normalize / fallback logic.
+ (WebCore::DOMWindow::scrollTo const): Make two-parameter version depends on one-parameter
+ version and rewrite the normalize / fallback logic.
+ * page/ScrollToOptions.h: Add <cmath> to use std::isfinite
+ (WebCore::ScrollToOptions::normalizeNonFiniteCoordinatesOrFallBackTo): New function to
+ normalize left/right values or fallback to the specified value if it is missing.
+
2018-08-06 Zan Dobersek <zdober...@igalia.com>
Unreviewed follow-up to r234594.
Modified: trunk/Source/WebCore/dom/Element.cpp (234595 => 234596)
--- trunk/Source/WebCore/dom/Element.cpp 2018-08-06 09:24:36 UTC (rev 234595)
+++ trunk/Source/WebCore/dom/Element.cpp 2018-08-06 09:56:08 UTC (rev 234596)
@@ -692,17 +692,15 @@
void Element::scrollBy(const ScrollToOptions& options)
{
- return scrollBy(options.left.value_or(0), options.top.value_or(0));
+ ScrollToOptions scrollToOptions = normalizeNonFiniteCoordinatesOrFallBackTo(options, 0, 0);
+ scrollToOptions.left.value() += scrollLeft();
+ scrollToOptions.top.value() += scrollTop();
+ scrollTo(scrollToOptions);
}
-static inline double normalizeNonFiniteValue(double f)
-{
- return std::isfinite(f) ? f : 0;
-}
-
void Element::scrollBy(double x, double y)
{
- scrollTo(scrollLeft() + normalizeNonFiniteValue(x), scrollTop() + normalizeNonFiniteValue(y));
+ scrollBy({ x, y });
}
void Element::scrollTo(const ScrollToOptions& options, ScrollClamping clamping)
@@ -720,12 +718,12 @@
if (!renderer || !renderer->hasOverflowClip())
return;
- // Normalize non-finite values for left and top dictionary members of options, if present.
- double x = options.left ? normalizeNonFiniteValue(options.left.value()) : adjustForAbsoluteZoom(renderer->scrollLeft(), *renderer);
- double y = options.top ? normalizeNonFiniteValue(options.top.value()) : adjustForAbsoluteZoom(renderer->scrollTop(), *renderer);
-
- renderer->setScrollLeft(clampToInteger(x * renderer->style().effectiveZoom()), clamping);
- renderer->setScrollTop(clampToInteger(y * renderer->style().effectiveZoom()), clamping);
+ ScrollToOptions scrollToOptions = normalizeNonFiniteCoordinatesOrFallBackTo(options,
+ adjustForAbsoluteZoom(renderer->scrollLeft(), *renderer),
+ adjustForAbsoluteZoom(renderer->scrollTop(), *renderer)
+ );
+ renderer->setScrollLeft(clampToInteger(scrollToOptions.left.value() * renderer->style().effectiveZoom()), clamping);
+ renderer->setScrollTop(clampToInteger(scrollToOptions.top.value() * renderer->style().effectiveZoom()), clamping);
}
void Element::scrollTo(double x, double y)
Modified: trunk/Source/WebCore/page/DOMWindow.cpp (234595 => 234596)
--- trunk/Source/WebCore/page/DOMWindow.cpp 2018-08-06 09:24:36 UTC (rev 234595)
+++ trunk/Source/WebCore/page/DOMWindow.cpp 2018-08-06 09:56:08 UTC (rev 234596)
@@ -1574,12 +1574,12 @@
return page->deviceScaleFactor();
}
-void DOMWindow::scrollBy(const ScrollToOptions& options) const
+void DOMWindow::scrollBy(double x, double y) const
{
- return scrollBy(options.left.value_or(0), options.top.value_or(0));
+ scrollBy({ x, y });
}
-void DOMWindow::scrollBy(double x, double y) const
+void DOMWindow::scrollBy(const ScrollToOptions& options) const
{
if (!isCurrentlyDisplayedInFrame())
return;
@@ -1590,29 +1590,17 @@
if (!view)
return;
- // Normalize non-finite values (https://drafts.csswg.org/cssom-view/#normalize-non-finite-values).
- x = std::isfinite(x) ? x : 0;
- y = std::isfinite(y) ? y : 0;
-
- IntSize scaledOffset(view->mapFromCSSToLayoutUnits(x), view->mapFromCSSToLayoutUnits(y));
+ ScrollToOptions scrollToOptions = normalizeNonFiniteCoordinatesOrFallBackTo(options, 0, 0);
+ IntSize scaledOffset(view->mapFromCSSToLayoutUnits(scrollToOptions.left.value()), view->mapFromCSSToLayoutUnits(scrollToOptions.top.value()));
view->setContentsScrollPosition(view->contentsScrollPosition() + scaledOffset);
}
-void DOMWindow::scrollTo(const ScrollToOptions& options, ScrollClamping clamping) const
+void DOMWindow::scrollTo(double x, double y, ScrollClamping clamping) const
{
- if (!isCurrentlyDisplayedInFrame())
- return;
-
- RefPtr<FrameView> view = m_frame->view();
- if (!view)
- return;
-
- double x = options.left ? options.left.value() : view->contentsScrollPosition().x();
- double y = options.top ? options.top.value() : view->contentsScrollPosition().y();
- return scrollTo(x, y, clamping);
+ scrollTo({ x, y }, clamping);
}
-void DOMWindow::scrollTo(double x, double y, ScrollClamping) const
+void DOMWindow::scrollTo(const ScrollToOptions& options, ScrollClamping) const
{
if (!isCurrentlyDisplayedInFrame())
return;
@@ -1621,16 +1609,16 @@
if (!view)
return;
- // Normalize non-finite values (https://drafts.csswg.org/cssom-view/#normalize-non-finite-values).
- x = std::isfinite(x) ? x : 0;
- y = std::isfinite(y) ? y : 0;
+ ScrollToOptions scrollToOptions = normalizeNonFiniteCoordinatesOrFallBackTo(options,
+ view->contentsScrollPosition().x(), view->contentsScrollPosition().y()
+ );
- if (!x && !y && view->contentsScrollPosition() == IntPoint(0, 0))
+ if (!scrollToOptions.left.value() && !scrollToOptions.top.value() && view->contentsScrollPosition() == IntPoint(0, 0))
return;
document()->updateLayoutIgnorePendingStylesheets();
- IntPoint layoutPos(view->mapFromCSSToLayoutUnits(x), view->mapFromCSSToLayoutUnits(y));
+ IntPoint layoutPos(view->mapFromCSSToLayoutUnits(scrollToOptions.left.value()), view->mapFromCSSToLayoutUnits(scrollToOptions.top.value()));
view->setContentsScrollPosition(layoutPos);
}
Modified: trunk/Source/WebCore/page/ScrollToOptions.h (234595 => 234596)
--- trunk/Source/WebCore/page/ScrollToOptions.h 2018-08-06 09:24:36 UTC (rev 234595)
+++ trunk/Source/WebCore/page/ScrollToOptions.h 2018-08-06 09:56:08 UTC (rev 234596)
@@ -28,6 +28,7 @@
#pragma once
+#include <cmath>
#include <wtf/Optional.h>
namespace WebCore {
@@ -37,4 +38,19 @@
std::optional<double> top;
};
+inline double normalizeNonFiniteValueOrFallBackTo(std::optional<double> value, double fallbackValue)
+{
+ // Normalize non-finite values (https://drafts.csswg.org/cssom-view/#normalize-non-finite-values).
+ return value ? (std::isfinite(*value) ? *value : 0) : fallbackValue;
}
+
+// FIXME(https://webkit.org/b/88339): Consider using FloatPoint or DoublePoint for fallback and return values.
+inline ScrollToOptions normalizeNonFiniteCoordinatesOrFallBackTo(const ScrollToOptions& value, double x, double y)
+{
+ ScrollToOptions options;
+ options.left = normalizeNonFiniteValueOrFallBackTo(value.left, x);
+ options.top = normalizeNonFiniteValueOrFallBackTo(value.top, y);
+ return options;
+}
+
+}
