Changeset 202132 in webkit

Timestamp:

Jun 16, 2016 11:49:14 AM (8 years ago)

Author:

Simon Fraser

Message:

[iOS WK2] On iPad, indirect focussing of a text field doesn't always scroll to the correct location
​https://bugs.webkit.org/show_bug.cgi?id=158828

Reviewed by Enrica Casucci.

Source/WebKit2:

WebPage::getAssistedNodeInformation() unconditionally set information.selectionRect()
to a 1x1 rect at the last interaction location. This caused -[WKWebView _zoomToFocusRect:...]
to early return under the !forceScroll clause if the last interaction rect was visible, but
the elementRect was not. This would happen, for example, if a click in a <button> focussed
an input on some other portion of the page. This behavior is iPad-only, because forceScroll
is only NO there.

Fix by making getAssistedNodeInformation() only set the selectionRect to the last interaction
location if that is inside of the elementRect (this caters to the case where a tap is inside
an input which is larger than the screen). Otherwise, set it to the empty rect.

Make this testable by allowing a test to opt into the iPad-style scrolling/zooming behavior
in this code path (essentially, making forceScroll NO for testing), via testing SPI on WKWebView,
plumbed through to WKContentView.

• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView forceIPadStyleZoomOnInputFocus]):
(-[WKWebView setForceIPadStyleZoomOnInputFocus:]):

• UIProcess/API/Cocoa/WKWebViewPrivate.h:
• UIProcess/ios/WKContentViewInteraction.h:
• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView _displayFormNodeInputView]):
(-[WKContentView requiresAccessoryView:]):
(-[WKContentView inputAccessoryView]):
(-[WKContentView forceIPadStyleZoomOnInputFocus]):
(-[WKContentView setForceIPadStyleZoomOnInputFocus:]):
(-[WKContentView requiresAccessoryView]): Remove redundant returns, and allow testing to
override the "isIPad" condition.

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::getAssistedNodeInformation):

Tools:

Expose "forceIPadStyleZoomOnInputFocus" on UIScriptController, which allows iPad-style
zooming behavior on <input> focus in the iPhone simulator, which is used for testing.

• WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:
• WebKitTestRunner/UIScriptContext/UIScriptController.cpp:

(WTR::UIScriptController::forceIPadStyleZoomOnInputFocus):
(WTR::UIScriptController::setForceIPadStyleZoomOnInputFocus):

• WebKitTestRunner/UIScriptContext/UIScriptController.h:
• WebKitTestRunner/ios/TestControllerIOS.mm:

(WTR::TestController::platformResetStateToConsistentValues):

• WebKitTestRunner/ios/UIScriptControllerIOS.mm:

(WTR::UIScriptController::forceIPadStyleZoomOnInputFocus):
(WTR::UIScriptController::setForceIPadStyleZoomOnInputFocus):

LayoutTests:

Add various tests for focusing form fields. focus-input-via-button-ipad.html tests
the change in the current patch.

• fast/forms/ios/focus-input-in-fixed-expected.txt: Added.
• fast/forms/ios/focus-input-in-fixed.html: Added.
• fast/forms/ios/focus-input-via-button-ipad-expected.txt: Added.
• fast/forms/ios/focus-input-via-button-ipad.html: Added.
• fast/forms/ios/focus-long-textarea-expected.txt: Added.
• fast/forms/ios/focus-long-textarea.html: Added.
• fast/forms/ios/resources/zooming-test-utils.js:

(testZoomAfterTap):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/ios/focus-input-in-fixed-expected.txt (added)
• LayoutTests/fast/forms/ios/focus-input-in-fixed.html (added)
• LayoutTests/fast/forms/ios/focus-input-via-button-ipad-expected.txt (added)
• LayoutTests/fast/forms/ios/focus-input-via-button-ipad.html (added)
• LayoutTests/fast/forms/ios/focus-long-textarea-expected.txt (added)
• LayoutTests/fast/forms/ios/focus-long-textarea.html (added)
• LayoutTests/fast/forms/ios/resources/zooming-test-utils.js (modified) (4 diffs)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (modified) (1 diff)
• Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h (modified) (1 diff)
• Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h (modified) (2 diffs)
• Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (modified) (6 diffs)
• Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl (modified) (1 diff)
• Tools/WebKitTestRunner/UIScriptContext/UIScriptController.cpp (modified) (1 diff)
• Tools/WebKitTestRunner/UIScriptContext/UIScriptController.h (modified) (1 diff)
• Tools/WebKitTestRunner/ios/TestControllerIOS.mm (modified) (2 diffs)
• Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-06-15  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] On iPad, indirect focussing of a text field doesn't always scroll to the correct location
+        https://bugs.webkit.org/show_bug.cgi?id=158828
+
+        Reviewed by Enrica Casucci.
+        
+        Add various tests for focusing form fields. focus-input-via-button-ipad.html tests
+        the change in the current patch.
+
+        * fast/forms/ios/focus-input-in-fixed-expected.txt: Added.
+        * fast/forms/ios/focus-input-in-fixed.html: Added.
+        * fast/forms/ios/focus-input-via-button-ipad-expected.txt: Added.
+        * fast/forms/ios/focus-input-via-button-ipad.html: Added.
+        * fast/forms/ios/focus-long-textarea-expected.txt: Added.
+        * fast/forms/ios/focus-long-textarea.html: Added.
+        * fast/forms/ios/resources/zooming-test-utils.js:
+        (testZoomAfterTap):
+
 2016-06-16  Adam Bergkvist  <adam.bergkvist@ericsson.com>
 

trunk/LayoutTests/fast/forms/ios/resources/zooming-test-utils.js

@@ -1 +1 @@
 
-function testZoomAfterTap(targetElement, xOffset, yOffset)
+function testZoomAfterTap(targetElement, xOffset, yOffset, useIPadBehavior)
 {
     if (!window.testRunner || !testRunner.runUIScript)
@@ -7 +7 @@
     var point = getPointInsideElement(targetElement, xOffset, yOffset);
 
-    var uiScript = zoomAfterSingleTapUIScript(point.x, point.y);
+    var uiScript = zoomAfterSingleTapUIScript(point.x, point.y, useIPadBehavior);
     testRunner.runUIScript(uiScript, function(result) {
         var results = tableFromJSON(result);
@@ -15 +15 @@
 }
 
-function zoomAfterSingleTapUIScript(x, y)
+function zoomAfterSingleTapUIScript(x, y, useIPadBehavior)
 {
     return `
@@ -35 +35 @@
             };
 
+            if (${useIPadBehavior})
+                uiController.forceIPadStyleZoomOnInputFocus = true;
             uiController.singleTapAtPoint(${x}, ${y}, function() {});
         })();`

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-06-15  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] On iPad, indirect focussing of a text field doesn't always scroll to the correct location
+        https://bugs.webkit.org/show_bug.cgi?id=158828
+
+        Reviewed by Enrica Casucci.
+        
+        WebPage::getAssistedNodeInformation() unconditionally set information.selectionRect()
+        to a 1x1 rect at the last interaction location. This caused -[WKWebView _zoomToFocusRect:...]
+        to early return under the !forceScroll clause if the last interaction rect was visible, but
+        the elementRect was not. This would happen, for example, if a click in a <button> focussed
+        an input on some other portion of the page. This behavior is iPad-only, because forceScroll
+        is only NO there.
+        
+        Fix by making getAssistedNodeInformation() only set the selectionRect to the last interaction
+        location if that is inside of the elementRect (this caters to the case where a tap is inside
+        an input which is larger than the screen). Otherwise, set it to the empty rect.
+        
+        Make this testable by allowing a test to opt into the iPad-style scrolling/zooming behavior
+        in this code path (essentially, making forceScroll NO for testing), via testing SPI on WKWebView,
+        plumbed through to WKContentView.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView forceIPadStyleZoomOnInputFocus]):
+        (-[WKWebView setForceIPadStyleZoomOnInputFocus:]):
+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _displayFormNodeInputView]):
+        (-[WKContentView requiresAccessoryView:]):
+        (-[WKContentView inputAccessoryView]):
+        (-[WKContentView forceIPadStyleZoomOnInputFocus]):
+        (-[WKContentView setForceIPadStyleZoomOnInputFocus:]):
+        (-[WKContentView requiresAccessoryView]): Remove redundant returns, and allow testing to
+        override the "isIPad" condition.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::getAssistedNodeInformation):
+
 2016-06-15  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm

@@ -4487 +4487 @@
 }
 
+- (BOOL)forceIPadStyleZoomOnInputFocus
+{
+    return [_contentView forceIPadStyleZoomOnInputFocus];
+}
+
+- (void)setForceIPadStyleZoomOnInputFocus:(BOOL)forceIPadStyleZoom
+{
+    [_contentView setForceIPadStyleZoomOnInputFocus:forceIPadStyleZoom];
+}
+
 #endif // PLATFORM(IOS)
 

trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h

@@ -256 +256 @@
 - (CGPoint)_convertPointFromViewToContents:(CGPoint)point WK_AVAILABLE(WK_MAC_TBA, WK_IOS_TBA);
 
-- (void)keyboardAccessoryBarNext;
-- (void)keyboardAccessoryBarPrevious;
+- (void)keyboardAccessoryBarNext WK_AVAILABLE(NA, WK_IOS_TBA);
+- (void)keyboardAccessoryBarPrevious WK_AVAILABLE(NA, WK_IOS_TBA);
+
+@property (nonatomic) BOOL forceIPadStyleZoomOnInputFocus WK_AVAILABLE(NA, WK_IOS_TBA);
 #endif
 

trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h

@@ -172 +172 @@
 
     BOOL _resigningFirstResponder;
+
+    // For testing.
+    BOOL _forceIPadStyleZoomOnInputFocus;
 }
 
@@ -232 +235 @@
 #endif
 
+@interface WKContentView (WKInteractionTesting)
+
+@property (nonatomic) BOOL forceIPadStyleZoomOnInputFocus;
+
+@end
+
 #endif // PLATFORM(IOS)

trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm

@@ -1068 +1068 @@
               minimumScale:_assistedNodeInformation.minimumScaleFactor
               maximumScale:_assistedNodeInformation.maximumScaleFactor
-              allowScaling:(_assistedNodeInformation.allowsUserScalingIgnoringForceAlwaysScaling && !UICurrentUserInterfaceIdiomIsPad())
-               forceScroll:[self requiresAccessoryView]];
+              allowScaling:(_assistedNodeInformation.allowsUserScalingIgnoringForceAlwaysScaling && (!UICurrentUserInterfaceIdiomIsPad() || _forceIPadStyleZoomOnInputFocus))
+               forceScroll:[self requiresAccessoryView:_forceIPadStyleZoomOnInputFocus]];
+
     _didAccessoryTabInitiateFocus = NO;
     [self _ensureFormAccessoryView];
@@ -1580 +1581 @@
 }
 
-- (BOOL)requiresAccessoryView
+- (BOOL)requiresAccessoryView:(BOOL)forceIPadBehavior
 {
     if ([_formInputSession accessoryViewShouldNotShow])
@@ -1596 +1597 @@
     case InputType::Number:
     case InputType::NumberPad:
-        return !UICurrentUserInterfaceIdiomIsPad();
     case InputType::ContentEditable:
     case InputType::TextArea:
-        return !UICurrentUserInterfaceIdiomIsPad();
     case InputType::Select:
     case InputType::Date:
@@ -1607 +1606 @@
     case InputType::Week:
     case InputType::Time:
-        return !UICurrentUserInterfaceIdiomIsPad();
+        return !(UICurrentUserInterfaceIdiomIsPad() || forceIPadBehavior);
     }
 }
@@ -1622 +1621 @@
 - (UIView *)inputAccessoryView
 {
-    if (![self requiresAccessoryView])
+    if (![self requiresAccessoryView:NO])
         return nil;
 
@@ -4113 +4112 @@
 #endif
 
+@implementation WKContentView (WKInteractionTesting)
+
+- (BOOL)forceIPadStyleZoomOnInputFocus
+{
+    return _forceIPadStyleZoomOnInputFocus;
+}
+
+- (void)setForceIPadStyleZoomOnInputFocus:(BOOL)forceIPadStyleZoom
+{
+    _forceIPadStyleZoomOnInputFocus = forceIPadStyleZoom;
+}
+
+@end
+
+
 // UITextRange, UITextPosition and UITextSelectionRect implementations for WK2
 

trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -2517 +2517 @@
     layoutIfNeeded();
 
-    // FIXME: This should return the selection rect, but when this is called at focus time
+    // FIXME: information.selectionRect should be set to the actual selection rect, but when this is called at focus time
     // we don't have a selection yet. Using the last interaction location is a reasonable approximation for now.
-    // FIXME: should the selection rect always be inside the elementRect?
     information.selectionRect = IntRect(m_lastInteractionLocation, IntSize(1, 1));
 
@@ -2539 +2538 @@
             frameView->setCustomFixedPositionLayoutRect(currentFixedPositionRect);
             
-            if (!information.elementRect.contains(information.selectionRect))
+            if (!information.elementRect.contains(m_lastInteractionLocation))
                 information.selectionRect.setLocation(information.elementRect.location());
+        } else {
+            // Don't use the selection rect if interaction was outside the element rect.
+            if (!information.elementRect.contains(m_lastInteractionLocation))
+                information.selectionRect = IntRect();
         }
     } else

trunk/Tools/ChangeLog

@@ +1 @@
+2016-06-15  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] On iPad, indirect focussing of a text field doesn't always scroll to the correct location
+        https://bugs.webkit.org/show_bug.cgi?id=158828
+
+        Reviewed by Enrica Casucci.
+        
+        Expose "forceIPadStyleZoomOnInputFocus" on UIScriptController, which allows iPad-style
+        zooming behavior on <input> focus in the iPhone simulator, which is used for testing.
+
+        * WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:
+        * WebKitTestRunner/UIScriptContext/UIScriptController.cpp:
+        (WTR::UIScriptController::forceIPadStyleZoomOnInputFocus):
+        (WTR::UIScriptController::setForceIPadStyleZoomOnInputFocus):
+        * WebKitTestRunner/UIScriptContext/UIScriptController.h:
+        * WebKitTestRunner/ios/TestControllerIOS.mm:
+        (WTR::TestController::platformResetStateToConsistentValues):
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::forceIPadStyleZoomOnInputFocus):
+        (WTR::UIScriptController::setForceIPadStyleZoomOnInputFocus):
+
 2016-06-15  Sam Weinig  <sam@webkit.org>
 

trunk/Tools/WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl

@@ -61 +61 @@
     readonly attribute object contentVisibleRect; // Returned object has 'left', 'top', 'width', 'height' properties.
 
+    // Behavior
+    attribute boolean forceIPadStyleZoomOnInputFocus;
+
     void uiScriptComplete(DOMString result);
 };

trunk/Tools/WebKitTestRunner/UIScriptContext/UIScriptController.cpp

@@ -179 +179 @@
 }
 
+bool UIScriptController::forceIPadStyleZoomOnInputFocus() const
+{
+    return false;
+}
+
+void UIScriptController::setForceIPadStyleZoomOnInputFocus(bool)
+{
+}
+
 void UIScriptController::platformSetWillBeginZoomingCallback()
 {

trunk/Tools/WebKitTestRunner/UIScriptContext/UIScriptController.h

@@ -81 +81 @@
 
     JSObjectRef contentVisibleRect() const;
+    
+    bool forceIPadStyleZoomOnInputFocus() const;
+    void setForceIPadStyleZoomOnInputFocus(bool);
 
     void uiScriptComplete(JSStringRef result);

trunk/Tools/WebKitTestRunner/ios/TestControllerIOS.mm

@@ -30 +30 @@
 #import "TestInvocation.h"
 #import <Foundation/Foundation.h>
+#import <UIKit/UIKit.h>
 #import <WebKit/WKPreferencesRefPrivate.h>
 #import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKStringCF.h>
 #import <WebKit/WKUserContentControllerPrivate.h>
-#import <WebKit/WKWebView.h>
-#import <WebKit/WKWebViewConfiguration.h>
 #import <WebKit/WKWebViewConfigurationPrivate.h>
+#import <WebKit/WKWebViewPrivate.h>
 #import <wtf/MainThread.h>
 
@@ -84 +84 @@
 {
     cocoaResetStateToConsistentValues();
+    [mainWebView()->platformView() setForceIPadStyleZoomOnInputFocus:NO];
 }
 

trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

@@ -210 +210 @@
 }
 
+bool UIScriptController::forceIPadStyleZoomOnInputFocus() const
+{
+    TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
+    return webView.forceIPadStyleZoomOnInputFocus;
+}
+
+void UIScriptController::setForceIPadStyleZoomOnInputFocus(bool value)
+{
+    TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
+    webView.forceIPadStyleZoomOnInputFocus = value;
+}
+
 void UIScriptController::platformSetWillBeginZoomingCallback()
 {