Changeset 243606 in webkit

Timestamp:

Mar 28, 2019 9:36:18 AM (5 years ago)

Author:

dbates@webkit.org

Message:

[iPad] Tapping on a popup form control may not show a popover
​https://bugs.webkit.org/show_bug.cgi?id=196322
<rdar://problem/49229632>

Reviewed by Wenson Hsieh.

Stop taking advantage of -[WKContentView inputView] being called when we invoke -reloadInputViews
to "lazily" allocate the input peripheral for the currently focused element. In theory, UIKit only
needs to call -inputView when it actually needs to display the input view (the keyboard). For
popup menu buttons, like <select>, no keyboard is needed. Instead we should create the peripheral
as part of the logic in the UI process to focus a new element before we call -reloadInputViews.

• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView inputView]): Extract logic to allocate the peripheral from here and moved it to createInputPeripheralWithView().
(-[WKContentView accessoryTab:]): While I am here, add a FIXME comment to explain why we need to
end the input sessions and nullify the input peripheral before we tell the web process to switch
focus as opposed to letting this happen after the web process tells us it focused a new element.
(createInputPeripheralWithView): Added.
(-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
Write in terms of createInputPeripheralWithView(). Create the input peripheral after becoming
first responder because creating the peripheral has known side-effects: for popup buttons it
tells the popup controller to present the popover. For key input to popovers to work from the get-go,
the content view must be the first responder. See <​https://bugs.webkit.org/show_bug.cgi?id=196272>
for more details.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ios/WKContentViewInteraction.mm (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-03-28  Daniel Bates  <dabates@apple.com>
+
+        [iPad] Tapping on a popup form control may not show a popover
+        https://bugs.webkit.org/show_bug.cgi?id=196322
+        <rdar://problem/49229632>
+
+        Reviewed by Wenson Hsieh.
+
+        Stop taking advantage of -[WKContentView inputView] being called when we invoke -reloadInputViews
+        to "lazily" allocate the input peripheral for the currently focused element. In theory, UIKit only
+        needs to call -inputView when it actually needs to display the input view (the keyboard). For
+        popup menu buttons, like <select>, no keyboard is needed. Instead we should create the peripheral
+        as part of the logic in the UI process to focus a new element before we call -reloadInputViews.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView inputView]): Extract logic to allocate the peripheral from here and moved it to createInputPeripheralWithView().
+        (-[WKContentView accessoryTab:]): While I am here, add a FIXME comment to explain why we need to
+        end the input sessions and nullify the input peripheral before we tell the web process to switch
+        focus as opposed to letting this happen after the web process tells us it focused a new element.
+        (createInputPeripheralWithView): Added.
+        (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
+        Write in terms of createInputPeripheralWithView(). Create the input peripheral after becoming
+        first responder because creating the peripheral has known side-effects: for popup buttons it
+        tells the popup controller to present the popover. For key input to popovers to work from the get-go,
+        the content view must be the first responder. See <https://bugs.webkit.org/show_bug.cgi?id=196272>
+        for more details.
+
 2019-03-28  Víctor Manuel Jáquez Leal  <vjaquez@igalia.com>
 

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

@@ -1643 +1643 @@
         return nil;
 
-    if (!_inputPeripheral) {
-        switch (_focusedElementInformation.elementType) {
-        case WebKit::InputType::Select:
-            _inputPeripheral = adoptNS([[WKFormSelectControl alloc] initWithView:self]);
-            break;
-#if ENABLE(INPUT_TYPE_COLOR)
-        case WebKit::InputType::Color:
-            _inputPeripheral = adoptNS([[WKFormColorControl alloc] initWithView:self]);
-            break;
-#endif
-        default:
-            _inputPeripheral = adoptNS([[WKFormInputControl alloc] initWithView:self]);
-            break;
-        }
-    } else {
+    if (_inputPeripheral) {
         // FIXME: UIKit may invoke -[WKContentView inputView] at any time when WKContentView is the first responder;
         // as such, it doesn't make sense to change the enclosing scroll view's zoom scale and content offset to reveal
@@ -1667 +1653 @@
         // rotation, when a select element is focused. See <https://webkit.org/b/192878> for more information.
         [self _zoomToRevealFocusedElement];
+
         [self _updateAccessory];
     }
@@ -3771 +3758 @@
 - (void)accessoryTab:(BOOL)isNext
 {
+    // The input peripheral may need to update the focused DOM node before we switch focus. The UI process does
+    // not maintain a handle to the actual focused DOM node – only the web process has such a handle. So, we need
+    // to end the editing session now before we tell the web process to switch focus. Once the web process tells
+    // us the newly focused element we are no longer are in a position to effect the previously focused element.
+    // See <https://bugs.webkit.org/show_bug.cgi?id=134409>.
     [self _endEditing];
-    _inputPeripheral = nil;
+    _inputPeripheral = nil; // Nullify so that we don't tell the input peripheral to end editing again in -_elementDidBlur.
 
     _isChangingFocusUsingAccessoryTab = YES;
@@ -4879 +4871 @@
 }
 
+static RetainPtr<NSObject <WKFormPeripheral>> createInputPeripheralWithView(WebKit::InputType type, WKContentView *view)
+{
+    switch (type) {
+    case WebKit::InputType::Select:
+        return adoptNS([[WKFormSelectControl alloc] initWithView:view]);
+#if ENABLE(INPUT_TYPE_COLOR)
+    case WebKit::InputType::Color:
+        return adoptNS([[WKFormColorControl alloc] initWithView:view]);
+#endif
+    default:
+        return adoptNS([[WKFormInputControl alloc] initWithView:view]);
+    }
+}
+
 - (void)_elementDidFocus:(const WebKit::FocusedElementInformation&)information userIsInteracting:(BOOL)userIsInteracting blurPreviousNode:(BOOL)blurPreviousNode changingActivityState:(BOOL)changingActivityState userObject:(NSObject <NSSecureCoding> *)userObject
 {
@@ -4979 +4985 @@
     BOOL editableChanged = [self setIsEditable:YES];
     _focusedElementInformation = information;
-    _inputPeripheral = nil;
     _traits = nil;
 
     if (![self isFirstResponder])
         [self becomeFirstResponder];
+
+    _inputPeripheral = createInputPeripheralWithView(_focusedElementInformation.elementType, self);
 
 #if PLATFORM(WATCHOS)