- Revision
- 288358
- Author
- mrobin...@webkit.org
- Date
- 2022-01-21 02:36:31 -0800 (Fri, 21 Jan 2022)
Log Message
element.scrollIntoView() sometimes doesn't scroll
https://bugs.webkit.org/show_bug.cgi?id=42593
Reviewed by Simon Fraser.
LayoutTests/imported/w3c:
* web-platform-tests/css/cssom-view/scrollIntoView-horizontal-partially-visible-expected.txt:
Source/WebCore:
No new tests. This is covered by existing WPT tests.
For some operations which scroll to a rectangle, if an object is more than
32 pixels onscreen, it's not considered onscreen. This was originally used
to prevent unnecessary scrolling while tabbing through form fields, but is
no longer used for that in the majority of cases. Instead, the behavior affects
the calls to Element.focus(), Element.scrollIntoView(), and navigations to
anchor elements.
While navigation to anchor elements and calls to Element.focus() offer more
flexibility to the user agent, this behavior is not spec-compliant with
scrollIntoView(). This change adds a flag to ScrollAlignment to turn it off.
It could be that, in the future, the behavior for focus() is specified more
thoroughly, which might mean extending this fix.
* dom/Element.cpp:
(WebCore::Element::scrollIntoView): Turn off the legacy horizontal visibility threshold.
(WebCore::Element::scrollIntoViewIfNeeded): Ditto.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::getRectToExpose const): Use the new setting in ScrollAlignment
and also rework the code to use the new methods on ScrollAlignment.
* rendering/ScrollAlignment.h:
(WebCore::ScrollAlignment::getVisibleBehavior const): Converted this to a method to match
the new ones.
(WebCore::ScrollAlignment::getPartialBehavior const): Ditto.
(WebCore::ScrollAlignment::getHiddenBehavior const): Ditto.
(WebCore::ScrollAlignment::disableLegacyHorizontalVisibilityThreshold): Added.
(WebCore::ScrollAlignment::legacyHorizontalVisibilityThresholdEnabled const): Added.
(WebCore::ScrollAlignment::getVisibleBehavior): Deleted.
(WebCore::ScrollAlignment::getPartialBehavior): Deleted.
(WebCore::ScrollAlignment::getHiddenBehavior): Deleted.
LayoutTests:
* platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-empty-inline-expected.txt: Removed.
* platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-shy-expected.txt:
* platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt:
* platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt:
Modified Paths
Removed Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (288357 => 288358)
--- trunk/LayoutTests/ChangeLog 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/ChangeLog 2022-01-21 10:36:31 UTC (rev 288358)
@@ -1,3 +1,15 @@
+2022-01-21 Martin Robinson <mrobin...@webkit.org>
+
+ element.scrollIntoView() sometimes doesn't scroll
+ https://bugs.webkit.org/show_bug.cgi?id=42593
+
+ Reviewed by Simon Fraser.
+
+ * platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-empty-inline-expected.txt: Removed.
+ * platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-shy-expected.txt:
+ * platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt:
+ * platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt:
+
2022-01-21 Kimmo Kinnunen <kkinnu...@apple.com>
[GLIB] Garden fast/mediastream/getUserMedia-to-canvas-*.html failure for WPE
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (288357 => 288358)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2022-01-21 10:36:31 UTC (rev 288358)
@@ -1,3 +1,12 @@
+2022-01-21 Martin Robinson <mrobin...@webkit.org>
+
+ element.scrollIntoView() sometimes doesn't scroll
+ https://bugs.webkit.org/show_bug.cgi?id=42593
+
+ Reviewed by Simon Fraser.
+
+ * web-platform-tests/css/cssom-view/scrollIntoView-horizontal-partially-visible-expected.txt:
+
2022-01-20 Alexey Shvayka <ashva...@apple.com>
Callback functions / interfaces should use global object of its _value_ for errors and lifecycle
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-partially-visible-expected.txt (288357 => 288358)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-partially-visible-expected.txt 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scrollIntoView-horizontal-partially-visible-expected.txt 2022-01-21 10:36:31 UTC (rev 288358)
@@ -1,3 +1,3 @@
-FAIL scrollIntoView scrolls partially-visible child in both axes assert_equals: Should have scrolled in the inline direction expected 200 but got 0
+PASS scrollIntoView scrolls partially-visible child in both axes
Deleted: trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-empty-inline-expected.txt (288357 => 288358)
--- trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-empty-inline-expected.txt 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-empty-inline-expected.txt 2022-01-21 10:36:31 UTC (rev 288358)
@@ -1,3 +0,0 @@
-
-FAIL getBoundingClientRect-empty-inline assert_equals: y expected 92 but got 0
-
Modified: trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-shy-expected.txt (288357 => 288358)
--- trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-shy-expected.txt 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/getBoundingClientRect-shy-expected.txt 2022-01-21 10:36:31 UTC (rev 288358)
@@ -14,5 +14,5 @@
PASS Collapsed soft-hyphen should be 0 width.
FAIL Rendered soft-hyphen should have a width. assert_equals: expected 10 but got 3
PASS Collapsed soft-hyphen in a span should be 0 width.
-FAIL Rendered soft-hyphen in a span should have a width. assert_equals: expected 10 but got 0
+FAIL Rendered soft-hyphen in a span should have a width. assert_equals: expected 10 but got 3
Modified: trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt (288357 => 288358)
--- trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-empty-inline-expected.txt 2022-01-21 10:36:31 UTC (rev 288358)
@@ -5,7 +5,7 @@
ref
-FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 0 assert_equals: offsetLeft expected 16 but got 0
+PASS offsetTop/Left of empty inline elements should work as if they were not empty: 0
FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 1 assert_equals: offsetLeft expected 34 but got 0
FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 2 assert_equals: offsetLeft expected 109 but got 0
Modified: trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt (288357 => 288358)
--- trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/cssom-view/offsetTopLeft-leading-space-inline-expected.txt 2022-01-21 10:36:31 UTC (rev 288358)
@@ -5,7 +5,7 @@
ref
-FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 0 assert_equals: offsetLeft expected 16 but got 0
+PASS offsetTop/Left of empty inline elements should work as if they were not empty: 0
FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 1 assert_equals: offsetLeft expected 34 but got 0
FAIL offsetTop/Left of empty inline elements should work as if they were not empty: 2 assert_equals: offsetLeft expected 109 but got 0
Modified: trunk/Source/WebCore/ChangeLog (288357 => 288358)
--- trunk/Source/WebCore/ChangeLog 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/Source/WebCore/ChangeLog 2022-01-21 10:36:31 UTC (rev 288358)
@@ -1,3 +1,43 @@
+2022-01-21 Martin Robinson <mrobin...@webkit.org>
+
+ element.scrollIntoView() sometimes doesn't scroll
+ https://bugs.webkit.org/show_bug.cgi?id=42593
+
+ Reviewed by Simon Fraser.
+
+ No new tests. This is covered by existing WPT tests.
+
+ For some operations which scroll to a rectangle, if an object is more than
+ 32 pixels onscreen, it's not considered onscreen. This was originally used
+ to prevent unnecessary scrolling while tabbing through form fields, but is
+ no longer used for that in the majority of cases. Instead, the behavior affects
+ the calls to Element.focus(), Element.scrollIntoView(), and navigations to
+ anchor elements.
+
+ While navigation to anchor elements and calls to Element.focus() offer more
+ flexibility to the user agent, this behavior is not spec-compliant with
+ scrollIntoView(). This change adds a flag to ScrollAlignment to turn it off.
+
+ It could be that, in the future, the behavior for focus() is specified more
+ thoroughly, which might mean extending this fix.
+
+ * dom/Element.cpp:
+ (WebCore::Element::scrollIntoView): Turn off the legacy horizontal visibility threshold.
+ (WebCore::Element::scrollIntoViewIfNeeded): Ditto.
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::getRectToExpose const): Use the new setting in ScrollAlignment
+ and also rework the code to use the new methods on ScrollAlignment.
+ * rendering/ScrollAlignment.h:
+ (WebCore::ScrollAlignment::getVisibleBehavior const): Converted this to a method to match
+ the new ones.
+ (WebCore::ScrollAlignment::getPartialBehavior const): Ditto.
+ (WebCore::ScrollAlignment::getHiddenBehavior const): Ditto.
+ (WebCore::ScrollAlignment::disableLegacyHorizontalVisibilityThreshold): Added.
+ (WebCore::ScrollAlignment::legacyHorizontalVisibilityThresholdEnabled const): Added.
+ (WebCore::ScrollAlignment::getVisibleBehavior): Deleted.
+ (WebCore::ScrollAlignment::getPartialBehavior): Deleted.
+ (WebCore::ScrollAlignment::getHiddenBehavior): Deleted.
+
2022-01-21 Fujii Hironori <hironori.fu...@sony.com>
Fix AppleWin build with newer MSVC
Modified: trunk/Source/WebCore/dom/Element.cpp (288357 => 288358)
--- trunk/Source/WebCore/dom/Element.cpp 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/Source/WebCore/dom/Element.cpp 2022-01-21 10:36:31 UTC (rev 288358)
@@ -1019,6 +1019,7 @@
auto writingMode = renderer()->style().writingMode();
ScrollAlignment alignX = toScrollAlignmentForInlineDirection(options.inlinePosition, writingMode, renderer()->style().isLeftToRightDirection());
ScrollAlignment alignY = toScrollAlignmentForBlockDirection(options.blockPosition, writingMode);
+ alignX.disableLegacyHorizontalVisibilityThreshold();
bool isHorizontal = renderer()->style().isHorizontalWritingMode();
ScrollRectToVisibleOptions visibleOptions {
@@ -1040,11 +1041,13 @@
bool insideFixed;
LayoutRect absoluteBounds = renderer()->absoluteAnchorRectWithScrollMargin(&insideFixed);
+
// Align to the top / bottom and to the closest edge.
- if (alignToTop)
- renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways, ShouldAllowCrossOriginScrolling::No });
- else
- renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignBottomAlways, ShouldAllowCrossOriginScrolling::No });
+ auto alignY = alignToTop ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignBottomAlways;
+ auto alignX = ScrollAlignment::alignToEdgeIfNeeded;
+ alignX.disableLegacyHorizontalVisibilityThreshold();
+
+ renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, alignX, alignY, ShouldAllowCrossOriginScrolling::No });
}
void Element::scrollIntoViewIfNeeded(bool centerIfNeeded)
@@ -1056,10 +1059,12 @@
bool insideFixed;
LayoutRect absoluteBounds = renderer()->absoluteAnchorRectWithScrollMargin(&insideFixed);
- if (centerIfNeeded)
- renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded, ShouldAllowCrossOriginScrolling::No });
- else
- renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded, ShouldAllowCrossOriginScrolling::No });
+
+ auto alignY = centerIfNeeded ? ScrollAlignment::alignCenterIfNeeded : ScrollAlignment::alignToEdgeIfNeeded;
+ auto alignX = centerIfNeeded ? ScrollAlignment::alignCenterIfNeeded : ScrollAlignment::alignToEdgeIfNeeded;
+ alignX.disableLegacyHorizontalVisibilityThreshold();
+
+ renderer()->scrollRectToVisible(absoluteBounds, insideFixed, { SelectionRevealMode::Reveal, alignX, alignY, ShouldAllowCrossOriginScrolling::No });
}
void Element::scrollIntoViewIfNotVisible(bool centerIfNotVisible)
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (288357 => 288358)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2022-01-21 10:36:31 UTC (rev 288358)
@@ -2624,21 +2624,21 @@
ScrollAlignment::Behavior scrollX;
LayoutRect exposeRectX(exposeRect.x(), visibleRect.y(), exposeRect.width(), visibleRect.height());
LayoutUnit intersectWidth = intersection(visibleRect, exposeRectX).width();
- if (intersectWidth == exposeRect.width() || intersectWidth >= MIN_INTERSECT_FOR_REVEAL)
+ if (intersectWidth == exposeRect.width() || (alignX.legacyHorizontalVisibilityThresholdEnabled() && intersectWidth >= MIN_INTERSECT_FOR_REVEAL)) {
// If the rectangle is fully visible, use the specified visible behavior.
// If the rectangle is partially visible, but over a certain threshold,
// then treat it as fully visible to avoid unnecessary horizontal scrolling
- scrollX = ScrollAlignment::getVisibleBehavior(alignX);
- else if (intersectWidth == visibleRect.width()) {
+ scrollX = alignX.getVisibleBehavior();
+ } else if (intersectWidth == visibleRect.width()) {
// If the rect is bigger than the visible area, don't bother trying to center. Other alignments will work.
- scrollX = ScrollAlignment::getVisibleBehavior(alignX);
+ scrollX = alignX.getVisibleBehavior();
if (scrollX == ScrollAlignment::Behavior::AlignCenter)
scrollX = ScrollAlignment::Behavior::NoScroll;
} else if (intersectWidth > 0)
// If the rectangle is partially visible, but not above the minimum threshold, use the specified partial behavior
- scrollX = ScrollAlignment::getPartialBehavior(alignX);
+ scrollX = alignX.getPartialBehavior();
else
- scrollX = ScrollAlignment::getHiddenBehavior(alignX);
+ scrollX = alignX.getHiddenBehavior();
// If we're trying to align to the closest edge, and the exposeRect is further right
// than the visibleRect, and not bigger than the visible area, then align with the right.
if (scrollX == ScrollAlignment::Behavior::AlignToClosestEdge && exposeRect.maxX() > visibleRect.maxX() && exposeRect.width() < visibleRect.width())
@@ -2661,17 +2661,17 @@
LayoutUnit intersectHeight = intersection(visibleRect, exposeRectY).height();
if (intersectHeight == exposeRect.height())
// If the rectangle is fully visible, use the specified visible behavior.
- scrollY = ScrollAlignment::getVisibleBehavior(alignY);
+ scrollY = alignY.getVisibleBehavior();
else if (intersectHeight == visibleRect.height()) {
// If the rect is bigger than the visible area, don't bother trying to center. Other alignments will work.
- scrollY = ScrollAlignment::getVisibleBehavior(alignY);
+ scrollY = alignY.getVisibleBehavior();
if (scrollY == ScrollAlignment::Behavior::AlignCenter)
scrollY = ScrollAlignment::Behavior::NoScroll;
} else if (intersectHeight > 0)
// If the rectangle is partially visible, use the specified partial behavior
- scrollY = ScrollAlignment::getPartialBehavior(alignY);
+ scrollY = alignY.getPartialBehavior();
else
- scrollY = ScrollAlignment::getHiddenBehavior(alignY);
+ scrollY = alignY.getHiddenBehavior();
// If we're trying to align to the closest edge, and the exposeRect is further down
// than the visibleRect, and not bigger than the visible area, then align with the bottom.
if (scrollY == ScrollAlignment::Behavior::AlignToClosestEdge && exposeRect.maxY() > visibleRect.maxY() && exposeRect.height() < visibleRect.height())
Modified: trunk/Source/WebCore/rendering/ScrollAlignment.h (288357 => 288358)
--- trunk/Source/WebCore/rendering/ScrollAlignment.h 2022-01-21 10:25:40 UTC (rev 288357)
+++ trunk/Source/WebCore/rendering/ScrollAlignment.h 2022-01-21 10:36:31 UTC (rev 288358)
@@ -61,9 +61,11 @@
AlignToClosestEdge
};
- static Behavior getVisibleBehavior(const ScrollAlignment& s) { return s.m_rectVisible; }
- static Behavior getPartialBehavior(const ScrollAlignment& s) { return s.m_rectPartial; }
- static Behavior getHiddenBehavior(const ScrollAlignment& s) { return s.m_rectHidden; }
+ Behavior getVisibleBehavior() const { return m_rectVisible; }
+ Behavior getPartialBehavior() const { return m_rectPartial; }
+ Behavior getHiddenBehavior() const { return m_rectHidden; }
+ void disableLegacyHorizontalVisibilityThreshold() { m_enableLegacyHorizontalVisibilityThreshold = false; }
+ bool legacyHorizontalVisibilityThresholdEnabled() const { return m_enableLegacyHorizontalVisibilityThreshold; }
static const ScrollAlignment alignCenterIfNotVisible;
static const ScrollAlignment alignToEdgeIfNotVisible;
@@ -78,6 +80,7 @@
Behavior m_rectVisible;
Behavior m_rectHidden;
Behavior m_rectPartial;
+ bool m_enableLegacyHorizontalVisibilityThreshold { true };
};
WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, ScrollAlignment::Behavior);
