Title: [191834] trunk
Revision
191834
Author
wenson_hs...@apple.com
Date
2015-10-30 18:09:10 -0700 (Fri, 30 Oct 2015)

Log Message

Inner height behavior when the keyboard is shown should match on WKWebView and MobileSafari
https://bugs.webkit.org/show_bug.cgi?id=150634
<rdar://problem/23202254>

Reviewed by Benjamin Poulain.

Source/WebKit2:

Make WKWebView match behavior in Safari by not firing resize events and changing the inner height when showing
or hiding the keyboard. Previously, the WKWebView would do both of the above because we use the scroll view's
contentInset property when no unobscured insets are explicitly set for the WKWebView. To fix this, when updating
the visible content rect of a WKWebView for computing the inner height, we readjust the computed bottom inset
to not take the keyboard height into account. To know how much we need to readjust the inset by, we store the
total amount by which the scroll view's bottom inset has been adjusted due to the keyboard.

We perform this readjustment in _updateVisibleContentRects rather than in _computedContentInset since some users
of _computedContentInset may still expect the bottom inset to account for the keyboard height. However, when
updating visible content rects, we should not account for the keyboard height since we don't want the inner height
to change when showing or hiding the keyboard.

Lastly, while calling _adjustForAutomaticKeyboardInfo, we may end up calling _updateVisibleContentRects. This call
is unnecessary since we call it again immediately after _adjustForAutomaticKeyboardInfo, and it also complicates
the readjustment logic, so it makes sense to prevent the update from happening altogether while we are adjusting
the scroll view's insets due to keyboard changes. Altogether, these changes mean that the computed inner height
will no longer be adjusted for the keyboard height on WKWebViews, matching the behavior observed on mobile Safari.

Test: LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize.html

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _updateVisibleContentRects]):
(-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):

LayoutTests:

Tests that WKWebViews do not fire resize events or change window.innerHeight when showing or
hiding the keyboard. This behavior is consistent with mobile Safari.

* fast/events/ios/keyboard-should-not-trigger-resize-expected.txt: Added.
* fast/events/ios/keyboard-should-not-trigger-resize.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (191833 => 191834)


--- trunk/LayoutTests/ChangeLog	2015-10-31 00:51:46 UTC (rev 191833)
+++ trunk/LayoutTests/ChangeLog	2015-10-31 01:09:10 UTC (rev 191834)
@@ -1,3 +1,17 @@
+2015-10-28  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Inner height behavior when the keyboard is shown should match on WKWebView and MobileSafari
+        https://bugs.webkit.org/show_bug.cgi?id=150634
+        <rdar://problem/23202254>
+
+        Reviewed by Benjamin Poulain.
+
+        Tests that WKWebViews do not fire resize events or change window.innerHeight when showing or
+        hiding the keyboard. This behavior is consistent with mobile Safari.
+
+        * fast/events/ios/keyboard-should-not-trigger-resize-expected.txt: Added.
+        * fast/events/ios/keyboard-should-not-trigger-resize.html: Added.
+
 2015-10-30  Brady Eidson  <beid...@apple.com>
 
         Modern IDB: Support IDBObjectStore.get() for IDBKeyRanges.

Added: trunk/LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize-expected.txt (0 => 191834)


--- trunk/LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize-expected.txt	2015-10-31 01:09:10 UTC (rev 191834)
@@ -0,0 +1,8 @@
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Before showing the keyboard, window.innerHeight = 480
+After showing the keyboard, window.innerHeight = 480
+After hiding the keyboard, window.innerHeight = 480
+

Added: trunk/LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize.html (0 => 191834)


--- trunk/LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize.html	2015-10-31 01:09:10 UTC (rev 191834)
@@ -0,0 +1,67 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+
+<html>
+
+<head>
+    <script src=""
+    <meta name="viewport" content="initial-scale=1.0, user-scalable=no">
+    <script id="ui-script" type="text/plain">
+        (function() {
+            uiController.didShowKeyboardCallback = function() {
+                uiController.typeCharacterUsingHardwareKeyboard("a", function() { });
+            }
+            uiController.didHideKeyboardCallback = function() {
+                uiController.uiScriptComplete();
+            }
+            uiController.singleTapAtPoint(50, 25, function() {});
+        })();
+    </script>
+
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        function getUIScript()
+        {
+            return document.getElementById("ui-script").text;
+        }
+
+        function runTest()
+        {
+            debug("Before showing the keyboard, window.innerHeight = " + window.innerHeight);
+            if (!window.testRunner || !testRunner.runUIScript)
+                return;
+
+            window._onresize_ = function() {
+                debug("Received unexpected onresize event! window.innerHeight = " + window.innerHeight);
+            }
+
+            testRunner.runUIScript(getUIScript(), function(result) {
+                debug("After hiding the keyboard, window.innerHeight = " + window.innerHeight);
+                testRunner.notifyDone();
+            });
+        }
+
+        function handleInput()
+        {
+            debug("After showing the keyboard, window.innerHeight = " + window.innerHeight);
+            document.getElementById("textfield").blur();
+        }
+    </script>
+    <style>
+    input {
+        width: 100px;
+        height: 50px;
+    }
+    </style>
+</head>
+
+<body style="margin: 0;" _onload_="runTest()">
+    <input id="textfield" style="width: 100px; height: 50px;" _oninput_="handleInput()">
+    <div id="console"></div>
+    <script src=""
+</body>
+
+</html>

Modified: trunk/Source/WebKit2/ChangeLog (191833 => 191834)


--- trunk/Source/WebKit2/ChangeLog	2015-10-31 00:51:46 UTC (rev 191833)
+++ trunk/Source/WebKit2/ChangeLog	2015-10-31 01:09:10 UTC (rev 191834)
@@ -1,3 +1,35 @@
+2015-10-28  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Inner height behavior when the keyboard is shown should match on WKWebView and MobileSafari
+        https://bugs.webkit.org/show_bug.cgi?id=150634
+        <rdar://problem/23202254>
+
+        Reviewed by Benjamin Poulain.
+
+        Make WKWebView match behavior in Safari by not firing resize events and changing the inner height when showing
+        or hiding the keyboard. Previously, the WKWebView would do both of the above because we use the scroll view's
+        contentInset property when no unobscured insets are explicitly set for the WKWebView. To fix this, when updating
+        the visible content rect of a WKWebView for computing the inner height, we readjust the computed bottom inset
+        to not take the keyboard height into account. To know how much we need to readjust the inset by, we store the
+        total amount by which the scroll view's bottom inset has been adjusted due to the keyboard.
+
+        We perform this readjustment in _updateVisibleContentRects rather than in _computedContentInset since some users
+        of _computedContentInset may still expect the bottom inset to account for the keyboard height. However, when
+        updating visible content rects, we should not account for the keyboard height since we don't want the inner height
+        to change when showing or hiding the keyboard.
+
+        Lastly, while calling _adjustForAutomaticKeyboardInfo, we may end up calling _updateVisibleContentRects. This call
+        is unnecessary since we call it again immediately after _adjustForAutomaticKeyboardInfo, and it also complicates
+        the readjustment logic, so it makes sense to prevent the update from happening altogether while we are adjusting
+        the scroll view's insets due to keyboard changes. Altogether, these changes mean that the computed inner height
+        will no longer be adjusted for the keyboard height on WKWebViews, matching the behavior observed on mobile Safari.
+
+        Test: LayoutTests/fast/events/ios/keyboard-should-not-trigger-resize.html
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _updateVisibleContentRects]):
+        (-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):
+
 2015-10-30  Anders Carlsson  <ander...@apple.com>
 
         Begin work on supporting reply blocks in _WKRemoteObjectRegistry

Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (191833 => 191834)


--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm	2015-10-31 00:51:46 UTC (rev 191833)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm	2015-10-31 01:09:10 UTC (rev 191834)
@@ -86,6 +86,7 @@
 #import <wtf/NeverDestroyed.h>
 #import <wtf/Optional.h>
 #import <wtf/RetainPtr.h>
+#import <wtf/TemporaryChange.h>
 
 #if PLATFORM(IOS)
 #import "_WKFrameHandleInternal.h"
@@ -212,6 +213,12 @@
 
     WebCore::Color _scrollViewBackgroundColor;
 
+    // This value tracks the current adjustment added to the bottom inset due to the keyboard sliding out from the bottom
+    // when computing obscured content insets. This is used when updating the visible content rects where we should not
+    // include this adjustment.
+    CGFloat _totalScrollViewBottomInsetAdjustmentForKeyboard;
+    BOOL _currentlyAdjustingScrollViewInsetsForKeyboard;
+
     BOOL _delayUpdateVisibleContentRects;
     BOOL _hadDelayedUpdateVisibleContentRects;
 
@@ -1671,19 +1678,20 @@
         return;
     }
 
-    if (_dynamicViewportUpdateMode != DynamicViewportUpdateMode::NotResizing)
+    if (_dynamicViewportUpdateMode != DynamicViewportUpdateMode::NotResizing
+        || _needsResetViewStateAfterCommitLoadForMainFrame
+        || [_scrollView isZoomBouncing]
+        || _currentlyAdjustingScrollViewInsetsForKeyboard)
         return;
 
-    if (_needsResetViewStateAfterCommitLoadForMainFrame)
-        return;
-
-    if ([_scrollView isZoomBouncing])
-        return;
-
     CGRect fullViewRect = self.bounds;
     CGRect visibleRectInContentCoordinates = _frozenVisibleContentRect ? _frozenVisibleContentRect.value() : [self convertRect:fullViewRect toView:_contentView.get()];
 
-    CGRect unobscuredRect = UIEdgeInsetsInsetRect(fullViewRect, [self _computedContentInset]);
+    UIEdgeInsets computedContentInsetUnadjustedForKeyboard = [self _computedContentInset];
+    if (!_haveSetObscuredInsets)
+        computedContentInsetUnadjustedForKeyboard.bottom -= _totalScrollViewBottomInsetAdjustmentForKeyboard;
+
+    CGRect unobscuredRect = UIEdgeInsetsInsetRect(fullViewRect, computedContentInsetUnadjustedForKeyboard);
     CGRect unobscuredRectInContentCoordinates = _frozenUnobscuredContentRect ? _frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()];
 
     CGFloat scaleFactor = contentZoomScale(self);
@@ -1699,7 +1707,7 @@
         WebKit::RemoteScrollingCoordinatorProxy* coordinator = _page->scrollingCoordinatorProxy();
         if (coordinator && coordinator->hasActiveSnapPoint()) {
             CGRect fullViewRect = self.bounds;
-            CGRect unobscuredRect = UIEdgeInsetsInsetRect(fullViewRect, [self _computedContentInset]);
+            CGRect unobscuredRect = UIEdgeInsetsInsetRect(fullViewRect, computedContentInsetUnadjustedForKeyboard);
             
             CGPoint currentPoint = [_scrollView contentOffset];
             CGPoint activePoint = coordinator->nearestActiveContentInsetAdjustedSnapPoint(unobscuredRect.origin.y, currentPoint);
@@ -1755,10 +1763,16 @@
     else
         _inputViewBounds = [self.window convertRect:CGRectIntersection([endFrameValue CGRectValue], self.window.screen.bounds) fromWindow:nil];
 
+    if (adjustScrollView) {
+        CGFloat bottomInsetBeforeAdjustment = [_scrollView contentInset].bottom;
+        TemporaryChange<BOOL> insetAdjustmentGuard(_currentlyAdjustingScrollViewInsetsForKeyboard, YES);
+        [_scrollView _adjustForAutomaticKeyboardInfo:keyboardInfo animated:YES lastAdjustment:&_lastAdjustmentForScroller];
+        CGFloat bottomInsetAfterAdjustment = [_scrollView contentInset].bottom;
+        if (bottomInsetBeforeAdjustment != bottomInsetAfterAdjustment)
+            _totalScrollViewBottomInsetAdjustmentForKeyboard += bottomInsetAfterAdjustment - bottomInsetBeforeAdjustment;
+    }
+
     [self _updateVisibleContentRects];
-
-    if (adjustScrollView)
-        [_scrollView _adjustForAutomaticKeyboardInfo:keyboardInfo animated:YES lastAdjustment:&_lastAdjustmentForScroller];
 }
 
 - (BOOL)_shouldUpdateKeyboardWithInfo:(NSDictionary *)keyboardInfo
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to