WebKit Bugzilla
Attachment 346469 Details for
Bug 188300
: Make two-arguments versions of scrollBy/scrollTo depend on the one-argument versions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-188300-20180803122854.patch (text/plain), 9.12 KB, created by
Frédéric Wang (:fredw)
on 2018-08-03 03:28:56 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Frédéric Wang (:fredw)
Created:
2018-08-03 03:28:56 PDT
Size:
9.12 KB
patch
obsolete
>Subversion Revision: 234540 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 0234a6a8eddfc436b79e7bfc25424045e4f0e2b5..ef0a3b03d79e3bcb5ad0b1b215437a9cca6bcf63 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,41 @@ >+2018-08-03 Frederic Wang <fwang@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 NOBODY (OOPS!). >+ >+ 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 moved to a >+ common ScrollToOptions::normalizeNonFiniteCoordinatesOrFallbackTo. Note that >+ DOMWindow::scrollTo has an optimization when scrolling to position (0, 0) but this is absent >+ from DOMWindow::scrollBy, so this patch does not merge their implementations in order to >+ avoid possible behavior change. >+ >+ 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 rely on normalizeNonFiniteCoordinatesOrFallbackTo. >+ (WebCore::Element::scrollTo): Rely on normalizeNonFiniteCoordinatesOrFallbackTo. >+ (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 rely on normalizeNonFiniteCoordinatesOrFallbackTo. >+ (WebCore::DOMWindow::scrollTo const): Ditto. >+ * page/ScrollToOptions.h: Add math.h 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-03 Carlos Garcia Campos <cgarcia@igalia.com> > > [WPE] Use WPE key symbols and new API instead of xkbcommon and the key mapper >diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp >index 4549e875e297846b33748b5d9ce737b178b90810..a3778021c073be1df43ae17d5d9efcddc274ac8b 100644 >--- a/Source/WebCore/dom/Element.cpp >+++ b/Source/WebCore/dom/Element.cpp >@@ -692,17 +692,16 @@ void Element::scrollIntoViewIfNotVisible(bool centerIfNotVisible) > > void Element::scrollBy(const ScrollToOptions& options) > { >- return scrollBy(options.left.value_or(0), options.top.value_or(0)); >-} >- >-static inline double normalizeNonFiniteValue(double f) >-{ >- return std::isfinite(f) ? f : 0; >+ ScrollToOptions scrollToOptions = options; >+ scrollToOptions.normalizeNonFiniteCoordinatesOrFallbackTo(0, 0); >+ scrollToOptions.left = scrollLeft() + scrollToOptions.left.value(); >+ scrollToOptions.top = scrollTop() + scrollToOptions.top.value(); >+ return scrollTo(scrollToOptions); > } > > 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 +719,13 @@ void Element::scrollTo(const ScrollToOptions& options, ScrollClamping clamping) > 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 = options; >+ scrollToOptions.normalizeNonFiniteCoordinatesOrFallbackTo( >+ 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) >diff --git a/Source/WebCore/page/DOMWindow.cpp b/Source/WebCore/page/DOMWindow.cpp >index 813fcaf55253075f19b8122cf01c6a9434833fb1..d4ebf217255d296122c21549455e80ea53178f84 100644 >--- a/Source/WebCore/page/DOMWindow.cpp >+++ b/Source/WebCore/page/DOMWindow.cpp >@@ -1574,12 +1574,12 @@ double DOMWindow::devicePixelRatio() const > 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)); >+ return scrollBy({ x, y }); > } > >-void DOMWindow::scrollBy(double x, double y) const >+void DOMWindow::scrollBy(const ScrollToOptions& options) const > { > if (!isCurrentlyDisplayedInFrame()) > return; >@@ -1590,29 +1590,18 @@ void DOMWindow::scrollBy(double x, double y) const > 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 = options; >+ scrollToOptions.normalizeNonFiniteCoordinatesOrFallbackTo(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); >+ return 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 +1610,18 @@ void DOMWindow::scrollTo(double x, double y, ScrollClamping) const > 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 = options; >+ scrollToOptions.normalizeNonFiniteCoordinatesOrFallbackTo( >+ 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); > } > >diff --git a/Source/WebCore/page/ScrollToOptions.h b/Source/WebCore/page/ScrollToOptions.h >index 84a9d9ddb2805d7f2b3943764a3190a9ccf5d0ff..7372a7bd4db2125a8026b66e768af64e07a898c4 100644 >--- a/Source/WebCore/page/ScrollToOptions.h >+++ b/Source/WebCore/page/ScrollToOptions.h >@@ -28,6 +28,7 @@ > > #pragma once > >+#include <math.h> > #include <wtf/Optional.h> > > namespace WebCore { >@@ -35,6 +36,13 @@ namespace WebCore { > struct ScrollToOptions { > std::optional<double> left; > std::optional<double> top; >+ >+ void normalizeNonFiniteCoordinatesOrFallbackTo(double x, double y) >+ { >+ // Normalize non-finite values (https://drafts.csswg.org/cssom-view/#normalize-non-finite-values). >+ left = left ? (std::isfinite(left.value()) ? left.value() : 0) : x; >+ top = top ? (std::isfinite(top.value()) ? top.value() : 0) : y; >+ }; > }; > > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 188300
:
346469
|
346512
|
346583
|
346616
|
346619