Layout viewport rect is too wide after window resize
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Oct 2017 20:27:18 +0000 (20:27 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Oct 2017 20:27:18 +0000 (20:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175235

Patch by Ali Juma <ajuma@chromium.org> on 2017-10-31
Reviewed by Dave Hyatt.

Source/WebCore:

After a window resize, ScrollView::updateScrollbars adds/removes scrollbars and triggers
layout. Each addition or removal triggers another pass, but at most 2 additional passes
are allowed. If a scrollbar is added or removed in the final allowed pass, layout is
left in an inconsistent state wrt the presence of scrollbars.

To avoid unnecessary passes, don't remove both scrollbars when only one needs to be
removed. This saves the extra pass needed to add the scrollbar back.

Test: fast/dom/Window/window-resize-update-scrollbars.html

* platform/ScrollView.cpp:
(WebCore::ScrollView::updateScrollbars):

LayoutTests:

* fast/dom/Window/window-resize-update-scrollbars-expected.txt: Added.
* fast/dom/Window/window-resize-update-scrollbars.html: Added.
* platform/ios/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224241 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/Window/window-resize-update-scrollbars-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Window/window-resize-update-scrollbars.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/ScrollView.cpp

index d99c7ca..795f686 100644 (file)
@@ -1,3 +1,14 @@
+2017-10-31  Ali Juma  <ajuma@chromium.org>
+
+        Layout viewport rect is too wide after window resize
+        https://bugs.webkit.org/show_bug.cgi?id=175235
+
+        Reviewed by Dave Hyatt.
+
+        * fast/dom/Window/window-resize-update-scrollbars-expected.txt: Added.
+        * fast/dom/Window/window-resize-update-scrollbars.html: Added.
+        * platform/ios/TestExpectations:
+
 2017-10-31  Youenn Fablet  <youenn@apple.com>
 
         Crash in:  com.apple.WebKit: WebKit::CacheStorage::Caches::initializeSize(WTF::Function<void (std::optional<WebCore::DOMCacheEngine::Error>&&)>&&) + 30 (CacheStorageEngineCaches.cpp:163)
diff --git a/LayoutTests/fast/dom/Window/window-resize-update-scrollbars-expected.txt b/LayoutTests/fast/dom/Window/window-resize-update-scrollbars-expected.txt
new file mode 100644 (file)
index 0000000..6ef73c3
--- /dev/null
@@ -0,0 +1,10 @@
+This test checks that the layout viewport rect's width is correctly updated when the window is resized.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.internals.layoutViewportRect().width is 585
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Window/window-resize-update-scrollbars.html b/LayoutTests/fast/dom/Window/window-resize-update-scrollbars.html
new file mode 100644 (file)
index 0000000..b24eeeb
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<style>
+    body {
+        height: 5000px;
+    }
+</style>
+<script src="../../../resources/js-test-pre.js"></script>
+<script>
+window.jsTestIsAsync = true;
+var widthDelta;
+
+function runTest() {
+    description("This test checks that the layout viewport rect's width is correctly updated when the window is resized.");
+    addEventListener("resize", resizeHandler);
+
+    widthDelta = window.outerWidth - window.internals.layoutViewportRect().width;
+    resizeTo(600, 500);
+}
+
+function resizeHandler() {
+    shouldEvaluateTo("window.internals.layoutViewportRect().width", 600 - widthDelta);
+    finishJSTest();
+}
+
+</script>
+<body onload="runTest()">
+<script src="../../../resources/js-test-post.js"></script>
+</body>
index 7e83f26..0252b4a 100644 (file)
@@ -1658,6 +1658,7 @@ fast/xsl/sort-locale.xml [ Failure ]
 
 # iOS does not support window resizing or window.resizeTo().
 fast/css-grid-layout/flex-content-sized-columns-resize.html [ Skip ]
+fast/dom/Window/window-resize-update-scrollbars.html [ Skip ]
 fast/dynamic/window-resize-scrollbars-test.html [ Skip ]
 fast/images/animated-gif-window-resizing.html [ Skip ]
 loader/go-back-to-different-window-size.html [ Skip ]
index 7e4d0d5..6ecbefd 100644 (file)
@@ -1,3 +1,23 @@
+2017-10-31  Ali Juma  <ajuma@chromium.org>
+
+        Layout viewport rect is too wide after window resize
+        https://bugs.webkit.org/show_bug.cgi?id=175235
+
+        Reviewed by Dave Hyatt.
+
+        After a window resize, ScrollView::updateScrollbars adds/removes scrollbars and triggers
+        layout. Each addition or removal triggers another pass, but at most 2 additional passes
+        are allowed. If a scrollbar is added or removed in the final allowed pass, layout is
+        left in an inconsistent state wrt the presence of scrollbars.
+
+        To avoid unnecessary passes, don't remove both scrollbars when only one needs to be
+        removed. This saves the extra pass needed to add the scrollbar back.
+
+        Test: fast/dom/Window/window-resize-update-scrollbars.html
+
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::updateScrollbars):
+
 2017-10-31  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [Attachment Support] Implement WKWebView SPI for inserting attachment elements
index 4b5a761..12f37fd 100644 (file)
@@ -607,7 +607,7 @@ void ScrollView::updateScrollbars(const ScrollPosition& desiredPosition)
 
         bool needAnotherPass = false;
         if (!hasOverlayScrollbars) {
-            // If we ever turn one scrollbar off, always turn the other one off too.  Never ever
+            // If we ever turn one scrollbar off, do not turn the other one on. Never ever
             // try to both gain/lose a scrollbar in the same pass.
             if (!m_updateScrollbarsPass && docSize.width() <= fullVisibleSize.width() && docSize.height() <= fullVisibleSize.height()) {
                 if (hScroll == ScrollbarAuto)
@@ -615,11 +615,11 @@ void ScrollView::updateScrollbars(const ScrollPosition& desiredPosition)
                 if (vScroll == ScrollbarAuto)
                     newHasVerticalScrollbar = false;
             }
-            if (!newHasHorizontalScrollbar && hasHorizontalScrollbar && vScroll != ScrollbarAlwaysOn) {
+            if (!newHasHorizontalScrollbar && hasHorizontalScrollbar && vScroll != ScrollbarAlwaysOn &&!hasVerticalScrollbar) {
                 newHasVerticalScrollbar = false;
                 needAnotherPass = true;
             }
-            if (!newHasVerticalScrollbar && hasVerticalScrollbar && hScroll != ScrollbarAlwaysOn) {
+            if (!newHasVerticalScrollbar && hasVerticalScrollbar && hScroll != ScrollbarAlwaysOn && !hasHorizontalScrollbar) {
                 newHasHorizontalScrollbar = false;
                 needAnotherPass = true;
             }