Overflow scrollIntoView wrong with borders

commit-queue@webkit.org · commit-queue@webkit.org · commit 4f1a6ea3bbc2 · 2020-10-27T21:04:15.000Z
https://bugs.webkit.org/show_bug.cgi?id=152660 Patch by Martin Robinson <mrobinson@igalia.com> on 2020-10-27 Reviewed by Simon Fraser. Source/WebCore: Tests: fast/overflow/scrollIntoView-nested-in-area-with-border.html * rendering/RenderLayer.cpp: (WebCore::RenderLayer::scrollRectToVisible): Offset the exposure rect by the border so that it is relative to the content rectangle. LayoutTests: Added a new test for this fix and renamed a similar test to follow a standard naming convention. This will make the two tests easier to find. * fast/overflow/scrollIntoView-nested-in-area-with-border-expected.html: Added. * fast/overflow/scrollIntoView-nested-in-area-with-border.html: Added. * fast/overflow/scrollIntoView-overflow-hidden-expected.html: Renamed from LayoutTests/fast/overflow/overflow-hidden-scroll-into-view-expected.html. * fast/overflow/scrollIntoView-overflow-hidden.html: Renamed from LayoutTests/fast/overflow/overflow-hidden-scroll-into-view.html. * fast/spatial-navigation/snav-div-overflow-scrol-hidden-expected.txt: Updated expectation. * platform/ios/TestExpectations: Updated test name in expectations. * platform/mac-wk2/editing/input/caret-at-the-edge-of-contenteditable-expected.txt: Updated expectation. * platform/mac-wk2/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt: Ditto. * platform/mac/editing/input/caret-at-the-edge-of-contenteditable-expected.txt: Ditto. * platform/mac/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt: Ditto. * platform/mac/editing/input/reveal-caret-of-multiline-input-expected.txt: Ditto. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@269070 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,25 @@
+2020-10-27  Martin Robinson  <mrobinson@igalia.com>
+
+        Overflow scrollIntoView wrong with borders
+        https://bugs.webkit.org/show_bug.cgi?id=152660
+
+        Reviewed by Simon Fraser.
+
+        Added a new test for this fix and renamed a similar test to follow a
+        standard naming convention. This will make the two tests easier to find.
+
+        * fast/overflow/scrollIntoView-nested-in-area-with-border-expected.html: Added.
+        * fast/overflow/scrollIntoView-nested-in-area-with-border.html: Added.
+        * fast/overflow/scrollIntoView-overflow-hidden-expected.html: Renamed from LayoutTests/fast/overflow/overflow-hidden-scroll-into-view-expected.html.
+        * fast/overflow/scrollIntoView-overflow-hidden.html: Renamed from LayoutTests/fast/overflow/overflow-hidden-scroll-into-view.html.
+        * fast/spatial-navigation/snav-div-overflow-scrol-hidden-expected.txt: Updated expectation.
+        * platform/ios/TestExpectations: Updated test name in expectations.
+        * platform/mac-wk2/editing/input/caret-at-the-edge-of-contenteditable-expected.txt: Updated expectation.
+        * platform/mac-wk2/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt: Ditto.
+        * platform/mac/editing/input/caret-at-the-edge-of-contenteditable-expected.txt: Ditto.
+        * platform/mac/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt: Ditto.
+        * platform/mac/editing/input/reveal-caret-of-multiline-input-expected.txt: Ditto.
+
 2020-10-27  Tim Horton  <timothy_horton@apple.com>
 
         REGRESSION (r265515): fast/events/touch/ios/touch-events-with-modifiers.html is a frequent failure
diff --git a/LayoutTests/fast/overflow/scrollIntoView-nested-in-area-with-border-expected.html b/LayoutTests/fast/overflow/scrollIntoView-nested-in-area-with-border-expected.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>Tests that scrollIntoView() works properly on a target nested inside a scrollable container with a border</title>
+    <style>
+        body {
+          margin: 0;
+        }
+        .box {
+          width: 50px;
+          height: 50px;
+          background: green;
+          margin: 100px;
+        }
+    </style>
+</head>
+
+<body>
+    <div class="box">
+</body>
+</html>
diff --git a/LayoutTests/fast/overflow/scrollIntoView-nested-in-area-with-border.html b/LayoutTests/fast/overflow/scrollIntoView-nested-in-area-with-border.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>Tests that scrollIntoView() works properly on a target nested inside a scrollable container with a border</title>
+    <style>
+        body {
+          margin: 0;
+        }
+        .container {
+          width: 100px;
+          height: 100px;
+          overflow: hidden;
+          border: solid rgba(0, 0, 0, 0) 100px;
+        }
+
+        .spacer {
+          height: 1000px;
+        }
+
+        #target {
+          width: 50px;
+          height: 50px;
+          background: green;
+        }
+    </style>
+</head>
+
+<body>
+    <div class="container">
+      <div class="spacer"></div>
+      <div id="target"></div>
+      <div class="spacer"></div>
+    </div>
+
+    <script>
+        function doTest() {
+            document.getElementById('target').scrollIntoView();
+        }
+        window.addEventListener('load', doTest, false);
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/fast/overflow/scrollIntoView-overflow-hidden-expected.html b/LayoutTests/fast/overflow/scrollIntoView-overflow-hidden-expected.html
diff --git a/LayoutTests/fast/overflow/scrollIntoView-overflow-hidden.html b/LayoutTests/fast/overflow/scrollIntoView-overflow-hidden.html
diff --git a/LayoutTests/fast/spatial-navigation/snav-div-overflow-scrol-hidden-expected.txt b/LayoutTests/fast/spatial-navigation/snav-div-overflow-scrol-hidden-expected.txt
@@ -11,8 +11,8 @@ PASS gFocusedDocument.activeElement.getAttribute("id") is "f2"
 PASS gFocusedDocument.activeElement.getAttribute("id") is "f3"
 PASS gFocusedDocument.activeElement.getAttribute("id") is "f3"
 PASS gFocusedDocument.activeElement.getAttribute("id") is "f4"
-FAIL gFocusedDocument.activeElement.getAttribute("id") should be f4. Was f5.
-FAIL gFocusedDocument.activeElement.getAttribute("id") should be f5. Was f6.
+PASS gFocusedDocument.activeElement.getAttribute("id") is "f4"
+PASS gFocusedDocument.activeElement.getAttribute("id") is "f5"
 PASS gFocusedDocument.activeElement.getAttribute("id") is "f6"
 PASS gFocusedDocument.activeElement.getAttribute("id") is "f9"
 This test is testing that div with overflow:auto would scroll, by div with overflow:hidden would not.
diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -2338,7 +2338,7 @@ fast/css/pseudo-visited-background-color-on-input.html [ ImageOnlyFailure ]
 fast/forms/select-size.html [ ImageOnlyFailure ]
 fast/inline/inline-with-outline-offset.html [ ImageOnlyFailure ]
 fast/inline/outline-corners-with-offset.html [ ImageOnlyFailure ]
-fast/overflow/overflow-hidden-scroll-into-view.html [ Pass ImageOnlyFailure ]
+fast/overflow/scrollIntoView-overflow-hidden.html [ Pass ImageOnlyFailure ]
 fast/sub-pixel/table-with-subpixel-cell-size.html [ ImageOnlyFailure ]
 svg/text/tspan-multiple-outline.svg [ ImageOnlyFailure ]
 fast/animation/height-auto-transition-computed-value.html [ ImageOnlyFailure ]
diff --git a/LayoutTests/platform/mac-wk2/editing/input/caret-at-the-edge-of-contenteditable-expected.txt b/LayoutTests/platform/mac-wk2/editing/input/caret-at-the-edge-of-contenteditable-expected.txt
@@ -7,7 +7,7 @@ layer at (0,0) size 800x72
         RenderText {#text} at (0,0) size 761x36
           text run at (0,0) width 761: "When the caret reaches the edge of the input box or content editable div, on the next input if must jump to the center of"
           text run at (0,18) width 73: "the control."
-layer at (8,44) size 82x20 clip at (9,45) size 80x18 scrollX 42 scrollWidth 337
+layer at (8,44) size 82x20 clip at (9,45) size 80x18 scrollX 41 scrollWidth 337
   RenderBlock {DIV} at (0,36) size 82x20 [border: (1px solid #000000)]
     RenderText {#text} at (1,1) size 336x18
       text run at (1,1) width 336: "012345678901012345678901234567890123456789"
diff --git a/LayoutTests/platform/mac-wk2/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt b/LayoutTests/platform/mac-wk2/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt
@@ -6,7 +6,7 @@ layer at (0,0) size 800x216
       RenderBlock {DIV} at (0,0) size 784x18
         RenderText {#text} at (0,0) size 610x18
           text run at (0,0) width 610: "When the caret is scrolled out, on starting typing it must be brought to the center of the control."
-layer at (8,26) size 784x182 clip at (9,27) size 767x165 scrollY 252 scrollHeight 558
+layer at (8,26) size 784x182 clip at (9,27) size 767x165 scrollY 251 scrollHeight 558
   RenderBlock {DIV} at (0,18) size 784x182 [border: (1px solid #000000)]
     RenderText {#text} at (1,1) size 16x18
       text run at (1,1) width 16: "00"
diff --git a/LayoutTests/platform/mac/editing/input/caret-at-the-edge-of-contenteditable-expected.txt b/LayoutTests/platform/mac/editing/input/caret-at-the-edge-of-contenteditable-expected.txt
@@ -7,7 +7,7 @@ layer at (0,0) size 800x72
         RenderText {#text} at (0,0) size 761x36
           text run at (0,0) width 761: "When the caret reaches the edge of the input box or content editable div, on the next input if must jump to the center of"
           text run at (0,18) width 73: "the control."
-layer at (8,44) size 82x20 clip at (9,45) size 80x18 scrollX 42 scrollWidth 337
+layer at (8,44) size 82x20 clip at (9,45) size 80x18 scrollX 41 scrollWidth 337
   RenderBlock {DIV} at (0,36) size 82x20 [border: (1px solid #000000)]
     RenderText {#text} at (1,1) size 336x18
       text run at (1,1) width 336: "012345678901012345678901234567890123456789"
diff --git a/LayoutTests/platform/mac/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt b/LayoutTests/platform/mac/editing/input/reveal-caret-of-multiline-contenteditable-expected.txt
@@ -6,7 +6,7 @@ layer at (0,0) size 800x216
       RenderBlock {DIV} at (0,0) size 784x18
         RenderText {#text} at (0,0) size 610x18
           text run at (0,0) width 610: "When the caret is scrolled out, on starting typing it must be brought to the center of the control."
-layer at (8,26) size 784x182 clip at (9,27) size 767x165 scrollY 252 scrollHeight 558
+layer at (8,26) size 784x182 clip at (9,27) size 767x165 scrollY 251 scrollHeight 558
   RenderBlock {DIV} at (0,18) size 784x182 [border: (1px solid #000000)]
     RenderText {#text} at (1,1) size 16x18
       text run at (1,1) width 16: "00"
diff --git a/LayoutTests/platform/mac/editing/input/reveal-caret-of-multiline-input-expected.txt b/LayoutTests/platform/mac/editing/input/reveal-caret-of-multiline-input-expected.txt
@@ -9,7 +9,7 @@ layer at (0,0) size 800x178
       RenderBlock (anonymous) at (0,18) size 784x144
         RenderText {#text} at (0,0) size 0x0
         RenderText {#text} at (0,0) size 0x0
-layer at (10,28) size 91x136 clip at (11,29) size 74x134 scrollY 99 scrollHeight 420
+layer at (10,28) size 91x136 clip at (11,29) size 74x134 scrollY 98 scrollHeight 420
   RenderTextControl {TEXTAREA} at (2,2) size 91x136 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     RenderBlock {DIV} at (3,3) size 70x416
       RenderText {#text} at (0,0) size 20x403
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2020-10-27  Martin Robinson  <mrobinson@igalia.com>
+
+        Overflow scrollIntoView wrong with borders
+        https://bugs.webkit.org/show_bug.cgi?id=152660
+
+        Reviewed by Simon Fraser.
+
+        Tests: fast/overflow/scrollIntoView-nested-in-area-with-border.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollRectToVisible): Offset the exposure rect by the border
+        so that it is relative to the content rectangle.
+
 2020-10-27  Lauro Moura  <lmoura@igalia.com>
 
         Unreviewed. Followup for r269058: Update bindings reference files
diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
@@ -2816,6 +2816,11 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& absoluteRect, bool insid
         RenderBox* box = renderBox();
         ASSERT(box);
         LayoutRect localExposeRect(box->absoluteToLocalQuad(FloatQuad(FloatRect(absoluteRect))).boundingBox());
+
+        // localExposedRect is now the absolute rect in local coordinates, but relative to the
+        // border edge. Make the rectangle relative to the scrollable area.
+        localExposeRect.moveBy(-LayoutPoint(box->borderLeft(), box->borderTop()));
+
         if (shouldPlaceBlockDirectionScrollbarOnLeft()) {
             // For direction: rtl; writing-mode: horizontal-tb box, the scroll bar is on the left side. The visible rect
             // starts from the right side of scroll bar. So the x of localExposeRect should start from the same position too.
