Changeset 230752 in webkit


Ignore:
Timestamp:
Apr 17, 2018 11:03:06 PM (6 years ago)
Author:
Wenson Hsieh
Message:

[Extra zoom mode] Programmatically changing focus when an element already has focus is a confusing experience
​https://bugs.webkit.org/show_bug.cgi?id=184635
<rdar://problem/39440642>

Reviewed by Tim Horton.

Source/WebKit:

Currently on iOS, we allow element focus to present UI if the keyboard is already shown. In extra zoom mode,
this would lead to a confusing experience when the focus form control overlay is disabled, since fullscreen
input view controllers are swapped out from underneath the user. Currently, this also puts the UI process into a
bad state where the focused form control overlay is active, but still hidden. This patch makes some tweaks to
input view controller handling in the UI process to address these issues, and also adds WebKitTestRunner support
for simulating interactions with select menus in extra zoom mode. See comments below for more detail.

Test: fast/events/extrazoom/change-focus-during-change-event.html

  • UIProcess/API/Cocoa/WKUIDelegatePrivate.h:

Add new SPI delegate hooks to notify the UI delegate when view controllers are presented or dismissed in extra
zoom mode. See -presentViewControllerForCurrentAssistedNode and -dismissAllInputViewControllers.

  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::takeBackgroundActivityTokenForFullscreenInput):
(WebKit::WebProcessProxy::releaseBackgroundActivityTokenForFullscreenInput):

See the comment below -dismissAllInputViewControllers.

  • UIProcess/WebProcessProxy.h:
  • UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):

In extra zoom mode, when changing focus from one assisted node to another, only allow the second node to be
assisted if the focused form control overlay is being shown. Otherwise, (i.e. when a fullscreen input view
controller is being presented), don't allow focus to start an input session.

Additionally, make a minor tweak to allow the previous node to blur, even if we are not showing the keyboard for
the new focused element. Without this adjustment, in the case where the page has programmatically focused
another element while a fullscreen input view controller is presented, we'll show the old view controller for
the new focused element.

(-[WKContentView presentViewControllerForCurrentAssistedNode]):
(-[WKContentView dismissAllInputViewControllers:]):

Currently, when a fullscreen input view controller is presented, the web process gets backgrounded. This
prevents event handlers from executing, which leads to strange behaviors in many cases (for instance: if we
have a multiple select, and the "change" event handler blurs the select, the user may check or uncheck multiple
items, but only the first change will actually take effect).

To fix this, we maintain a background activity token while presenting an input view controller.

(-[WKContentView focusedFormControlViewDidBeginEditing:]):

Start hiding the focused form overlay when re-presenting an input view controller. This allows us to bail from
showing fullscreen input UI for another focused element if focus programmatically changes while the current
fullscreen input view controller is presented, due to the -isHidden check in -_startAssistingNode:.

(-[WKContentView selectFormAccessoryPickerRow:]):

Simulate tapping a given row in select menu UI in extra zoom mode.

Tools:

Add plumbing to support invoking didHideKeyboardCallback and didShowKeyboardCallback when (respectively)
dismissing or presenting fullscreen input view controllers in extra zoom mode.

  • WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:

(-[TestRunnerWKWebView initWithFrame:configuration:]):
(-[TestRunnerWKWebView dealloc]):
(-[TestRunnerWKWebView _invokeShowKeyboardCallbackIfNecessary]):
(-[TestRunnerWKWebView _invokeHideKeyboardCallbackIfNecessary]):
(-[TestRunnerWKWebView _keyboardDidShow:]):
(-[TestRunnerWKWebView _keyboardDidHide:]):
(-[TestRunnerWKWebView _webView:didPresentFocusedElementViewController:]):
(-[TestRunnerWKWebView _webView:didDismissFocusedElementViewController:]):

LayoutTests:

Add a new layout test to exercise the following sequence of events in extra zoom mode:

  1. Focus select element #1.
  2. Choose an unselected option.
  3. Programmatically focus select element #2 in the "change" event handler.
  4. Choose an unselected option.
  5. Programmatically blur select element #2 in the "change" event handler.
  • fast/events/extrazoom/change-focus-during-change-event-expected.txt: Added.
  • fast/events/extrazoom/change-focus-during-change-event.html: Added.
  • resources/ui-helper.js:

(window.UIHelper.waitForKeyboardToHide.return.new.Promise):
(window.UIHelper.waitForKeyboardToHide):

Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r230746 r230752  
     12018-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Extra zoom mode] Programmatically changing focus when an element already has focus is a confusing experience
     4        https://bugs.webkit.org/show_bug.cgi?id=184635
     5        <rdar://problem/39440642>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a new layout test to exercise the following sequence of events in extra zoom mode:
     10
     11        1. Focus select element #1.
     12        2. Choose an unselected option.
     13        3. Programmatically focus select element #2 in the "change" event handler.
     14        4. Choose an unselected option.
     15        5. Programmatically blur select element #2 in the "change" event handler.
     16
     17        * fast/events/extrazoom/change-focus-during-change-event-expected.txt: Added.
     18        * fast/events/extrazoom/change-focus-during-change-event.html: Added.
     19        * resources/ui-helper.js:
     20        (window.UIHelper.waitForKeyboardToHide.return.new.Promise):
     21        (window.UIHelper.waitForKeyboardToHide):
     22
    1232018-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
    224

  • trunk/LayoutTests/resources/ui-helper.js

    r230527 r230752  
    101101    }
    102102
     103    static waitForKeyboardToHide()
     104    {
     105        return new Promise(resolve => {
     106            testRunner.runUIScript(`
     107                (function() {
     108                    uiController.didHideKeyboardCallback = () => uiController.uiScriptComplete();
     109                })()`, resolve);
     110        });
     111    }
     112
    103113    static getUICaretRect()
    104114    {

  • trunk/Source/WebKit/ChangeLog

    r230747 r230752  
     12018-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Extra zoom mode] Programmatically changing focus when an element already has focus is a confusing experience
     4        https://bugs.webkit.org/show_bug.cgi?id=184635
     5        <rdar://problem/39440642>
     6
     7        Reviewed by Tim Horton.
     8
     9        Currently on iOS, we allow element focus to present UI if the keyboard is already shown. In extra zoom mode,
     10        this would lead to a confusing experience when the focus form control overlay is disabled, since fullscreen
     11        input view controllers are swapped out from underneath the user. Currently, this also puts the UI process into a
     12        bad state where the focused form control overlay is active, but still hidden. This patch makes some tweaks to
     13        input view controller handling in the UI process to address these issues, and also adds WebKitTestRunner support
     14        for simulating interactions with select menus in extra zoom mode. See comments below for more detail.
     15
     16        Test: fast/events/extrazoom/change-focus-during-change-event.html
     17
     18        * UIProcess/API/Cocoa/WKUIDelegatePrivate.h:
     19
     20        Add new SPI delegate hooks to notify the UI delegate when view controllers are presented or dismissed in extra
     21        zoom mode. See -presentViewControllerForCurrentAssistedNode and -dismissAllInputViewControllers.
     22
     23        * UIProcess/WebProcessProxy.cpp:
     24        (WebKit::WebProcessProxy::takeBackgroundActivityTokenForFullscreenInput):
     25        (WebKit::WebProcessProxy::releaseBackgroundActivityTokenForFullscreenInput):
     26
     27        See the comment below -dismissAllInputViewControllers.
     28
     29        * UIProcess/WebProcessProxy.h:
     30        * UIProcess/ios/WKContentViewInteraction.mm:
     31        (-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
     32
     33        In extra zoom mode, when changing focus from one assisted node to another, only allow the second node to be
     34        assisted if the focused form control overlay is being shown. Otherwise, (i.e. when a fullscreen input view
     35        controller is being presented), don't allow focus to start an input session.
     36
     37        Additionally, make a minor tweak to allow the previous node to blur, even if we are not showing the keyboard for
     38        the new focused element. Without this adjustment, in the case where the page has programmatically focused
     39        another element while a fullscreen input view controller is presented, we'll show the old view controller for
     40        the new focused element.
     41
     42        (-[WKContentView presentViewControllerForCurrentAssistedNode]):
     43        (-[WKContentView dismissAllInputViewControllers:]):
     44
     45        Currently, when a fullscreen input view controller is presented, the web process gets backgrounded. This
     46        prevents event handlers from executing, which leads to strange behaviors in many cases (for instance: if we
     47        have a multiple select, and the "change" event handler blurs the select, the user may check or uncheck multiple
     48        items, but only the first change will actually take effect).
     49
     50        To fix this, we maintain a background activity token while presenting an input view controller.
     51
     52        (-[WKContentView focusedFormControlViewDidBeginEditing:]):
     53
     54        Start hiding the focused form overlay when re-presenting an input view controller. This allows us to bail from
     55        showing fullscreen input UI for another focused element if focus programmatically changes while the current
     56        fullscreen input view controller is presented, due to the -isHidden check in -_startAssistingNode:.
     57
     58        (-[WKContentView selectFormAccessoryPickerRow:]):
     59
     60        Simulate tapping a given row in select menu UI in extra zoom mode.
     61
    1622018-04-17  Conrad Shultz  <conrad_shultz@apple.com>
    263

  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKUIDelegatePrivate.h

    r230121 r230752  
    154154#endif
    155155- (void)_webView:(WKWebView *)webView didChangeSafeAreaShouldAffectObscuredInsets:(BOOL)safeAreaShouldAffectObscuredInsets WK_API_AVAILABLE(ios(11.0));
     156- (void)_webView:(WKWebView *)webView didPresentFocusedElementViewController:(UIViewController *)controller WK_API_AVAILABLE(ios(WK_IOS_TBA));
     157- (void)_webView:(WKWebView *)webView didDismissFocusedElementViewController:(UIViewController *)controller WK_API_AVAILABLE(ios(WK_IOS_TBA));
    156158#else // TARGET_OS_IPHONE
    157159- (void)_prepareForImmediateActionAnimationForWebView:(WKWebView *)webView WK_API_AVAILABLE(macosx(10.13.4));

  • trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

    r230640 r230752  
    14081408}
    14091409
     1410#if ENABLE(EXTRA_ZOOM_MODE)
     1411
     1412void WebProcessProxy::takeBackgroundActivityTokenForFullscreenInput()
     1413{
     1414    if (m_backgroundActivityTokenForFullscreenFormControls)
     1415        return;
     1416
     1417    m_backgroundActivityTokenForFullscreenFormControls = m_throttler.backgroundActivityToken();
     1418    RELEASE_LOG(ProcessSuspension, "UIProcess is taking a background assertion because it is presenting fullscreen UI for form controls.");
     1419}
     1420
     1421void WebProcessProxy::releaseBackgroundActivityTokenForFullscreenInput()
     1422{
     1423    if (!m_backgroundActivityTokenForFullscreenFormControls)
     1424        return;
     1425
     1426    m_backgroundActivityTokenForFullscreenFormControls = nullptr;
     1427    RELEASE_LOG(ProcessSuspension, "UIProcess is releasing a background assertion because it has dismissed fullscreen UI for form controls.");
     1428}
     1429
     1430#endif
     1431
    14101432} // namespace WebKit

  • trunk/Source/WebKit/UIProcess/WebProcessProxy.h

    r230640 r230752  
    213213    void suspendedPageWasDestroyed(SuspendedPageProxy&);
    214214
     215#if ENABLE(EXTRA_ZOOM_MODE)
     216    void takeBackgroundActivityTokenForFullscreenInput();
     217    void releaseBackgroundActivityTokenForFullscreenInput();
     218#endif
     219
    215220protected:
    216221    static uint64_t generatePageID();
    … …  
    341346
    342347    bool m_hasCommittedAnyProvisionalLoads { false };
     348
     349#if ENABLE(EXTRA_ZOOM_MODE)
     350    ProcessThrottler::BackgroundActivityToken m_backgroundActivityTokenForFullscreenFormControls;
     351#endif
    343352};
    344353

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

    r230745 r230752  
    40124012        shouldShowKeyboard = [inputDelegate _webView:_webView focusShouldStartInputSession:focusedElementInfo.get()];
    40134013    else {
    4014         // The default behavior is to allow node assistance if the user is interacting or the keyboard is already active.
    4015         shouldShowKeyboard = userIsInteracting || _isChangingFocus || changingActivityState;
    4016 #if ENABLE(DATA_INTERACTION)
    4017         shouldShowKeyboard |= _dragDropInteractionState.isPerformingDrop();
     4014        // The default behavior is to allow node assistance if the user is interacting.
     4015        // We also allow node assistance if the keyboard already is showing, unless we're in extra zoom mode.
     4016        shouldShowKeyboard = userIsInteracting
     4017#if ENABLE(EXTRA_ZOOM_MODE)
     4018            || (_isChangingFocus && ![_focusedFormControlView isHidden])
     4019#else
     4020            || _isChangingFocus
    40184021#endif
    4019     }
    4020     if (!shouldShowKeyboard)
    4021         return;
     4022#if ENABLE(DRAG_SUPPORT)
     4023            || _dragDropInteractionState.isPerformingDrop()
     4024#endif
     4025            || changingActivityState;
     4026    }
    40224027
    40234028    if (blurPreviousNode)
    40244029        [self _stopAssistingNode];
     4030
     4031    if (!shouldShowKeyboard)
     4032        return;
    40254033
    40264034    if (!isAssistableInputType(information.elementType))
    … …  
    42174225    else
    42184226        [presentingViewController presentViewController:_presentedFullScreenInputViewController.get() animated:YES completion:nil];
     4227
     4228    // Presenting a fullscreen input view controller fully obscures the web view. Without taking this token, the web content process will get backgrounded.
     4229    _page->process().takeBackgroundActivityTokenForFullscreenInput();
     4230
     4231    [presentingViewController.transitionCoordinator animateAlongsideTransition:nil completion:[weakWebView = WeakObjCPtr<WKWebView>(_webView), controller = _presentedFullScreenInputViewController] (id <UIViewControllerTransitionCoordinatorContext>) {
     4232        auto strongWebView = weakWebView.get();
     4233        id <WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([strongWebView UIDelegate]);
     4234        if ([uiDelegate respondsToSelector:@selector(_webView:didPresentFocusedElementViewController:)])
     4235            [uiDelegate _webView:strongWebView.get() didPresentFocusedElementViewController:controller.get()];
     4236    }];
    42194237}
    42204238
    … …  
    42234241    auto navigationController = WTFMove(_inputNavigationViewControllerForFullScreenInputs);
    42244242    auto presentedController = WTFMove(_presentedFullScreenInputViewController);
     4243
     4244    if (!presentedController)
     4245        return;
    42254246
    42264247    if ([navigationController viewControllers].lastObject == presentedController.get())
    … …  
    42294250        [presentedController dismissViewControllerAnimated:animated completion:nil];
    42304251
     4252    [[presentedController transitionCoordinator] animateAlongsideTransition:nil completion:[weakWebView = WeakObjCPtr<WKWebView>(_webView), controller = presentedController] (id <UIViewControllerTransitionCoordinatorContext>) {
     4253        auto strongWebView = weakWebView.get();
     4254        id <WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([strongWebView UIDelegate]);
     4255        if ([uiDelegate respondsToSelector:@selector(_webView:didDismissFocusedElementViewController:)])
     4256            [uiDelegate _webView:strongWebView.get() didDismissFocusedElementViewController:controller.get()];
     4257    }];
     4258
    42314259    if (_shouldRestoreFirstResponderStatusAfterLosingFocus) {
    42324260        _shouldRestoreFirstResponderStatusAfterLosingFocus = NO;
    … …  
    42344262            [self becomeFirstResponder];
    42354263    }
     4264
     4265    _page->process().releaseBackgroundActivityTokenForFullscreenInput();
    42364266}
    42374267
    … …  
    42504280{
    42514281    [self updateCurrentAssistedNodeInformation:[weakSelf = WeakObjCPtr<WKContentView>(self)] (bool didUpdate) {
    4252         if (didUpdate)
    4253             [weakSelf presentViewControllerForCurrentAssistedNode];
     4282        if (!didUpdate)
     4283            return;
     4284
     4285        auto strongSelf = weakSelf.get();
     4286        [strongSelf presentViewControllerForCurrentAssistedNode];
     4287        [strongSelf->_focusedFormControlView hide:YES];
    42544288    }];
    42554289}
    … …  
    54025436- (void)selectFormAccessoryPickerRow:(NSInteger)rowIndex
    54035437{
     5438#if ENABLE(EXTRA_ZOOM_MODE)
     5439    if ([_presentedFullScreenInputViewController isKindOfClass:[WKSelectMenuListViewController class]])
     5440        [(WKSelectMenuListViewController *)_presentedFullScreenInputViewController.get() selectItemAtIndex:rowIndex];
     5441#else
    54045442    if ([_inputPeripheral isKindOfClass:[WKFormSelectControl self]])
    54055443        [(WKFormSelectControl *)_inputPeripheral selectRow:rowIndex inComponent:0 extendingSelection:NO];
     5444#endif
    54065445}
    54075446

  • trunk/Tools/ChangeLog

    r230749 r230752  
     12018-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Extra zoom mode] Programmatically changing focus when an element already has focus is a confusing experience
     4        https://bugs.webkit.org/show_bug.cgi?id=184635
     5        <rdar://problem/39440642>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add plumbing to support invoking `didHideKeyboardCallback` and `didShowKeyboardCallback` when (respectively)
     10        dismissing or presenting fullscreen input view controllers in extra zoom mode.
     11
     12        * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
     13        (-[TestRunnerWKWebView initWithFrame:configuration:]):
     14        (-[TestRunnerWKWebView dealloc]):
     15        (-[TestRunnerWKWebView _invokeShowKeyboardCallbackIfNecessary]):
     16        (-[TestRunnerWKWebView _invokeHideKeyboardCallbackIfNecessary]):
     17        (-[TestRunnerWKWebView _keyboardDidShow:]):
     18        (-[TestRunnerWKWebView _keyboardDidHide:]):
     19        (-[TestRunnerWKWebView _webView:didPresentFocusedElementViewController:]):
     20        (-[TestRunnerWKWebView _webView:didDismissFocusedElementViewController:]):
     21
    1222018-04-17  Michael Catanzaro  <mcatanzaro@igalia.com>
    223

  • trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm

    r227563 r230752  
    2828
    2929#import "WebKitTestRunnerDraggingInfo.h"
     30#import <WebKit/WKUIDelegatePrivate.h>
    3031#import <wtf/Assertions.h>
    3132#import <wtf/RetainPtr.h>
    … …  
    4748#if WK_API_ENABLED
    4849
    49 @interface TestRunnerWKWebView () {
     50@interface TestRunnerWKWebView () <WKUIDelegatePrivate> {
    5051    RetainPtr<NSNumber *> m_stableStateOverride;
    5152}
    5253
    5354@property (nonatomic, copy) void (^zoomToScaleCompletionHandler)(void);
    54 @property (nonatomic, copy) void (^showKeyboardCompletionHandler)(void);
    5555@property (nonatomic, copy) void (^retrieveSpeakSelectionContentCompletionHandler)(void);
    5656@property (nonatomic) BOOL isShowingKeyboard;
    … …  
    7373    if (self = [super initWithFrame:frame configuration:configuration]) {
    7474        NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
    75         [center addObserver:self selector:@selector(_keyboardDidShow:) name:UIKeyboardDidShowNotification object:nil];
    76         [center addObserver:self selector:@selector(_keyboardDidHide:) name:UIKeyboardDidHideNotification object:nil];
     75        [center addObserver:self selector:@selector(_invokeShowKeyboardCallbackIfNecessary) name:UIKeyboardDidShowNotification object:nil];
     76        [center addObserver:self selector:@selector(_invokeHideKeyboardCallbackIfNecessary) name:UIKeyboardDidHideNotification object:nil];
     77
     78        self.UIDelegate = self;
    7779    }
    7880    return self;
    … …  
    9597
    9698    self.zoomToScaleCompletionHandler = nil;
    97     self.showKeyboardCompletionHandler = nil;
    9899    self.retrieveSpeakSelectionContentCompletionHandler = nil;
    99100
    … …  
    140141}
    141142
    142 - (void)_keyboardDidShow:(NSNotification *)notification
     143- (void)_invokeShowKeyboardCallbackIfNecessary
    143144{
    144145    if (self.isShowingKeyboard)
    … …  
    150151}
    151152
    152 - (void)_keyboardDidHide:(NSNotification *)notification
     153- (void)_invokeHideKeyboardCallbackIfNecessary
    153154{
    154155    if (!self.isShowingKeyboard)
    … …  
    232233}
    233234
    234 #endif
     235#pragma mark - WKUIDelegatePrivate
     236
     237// In extra zoom mode, fullscreen form control UI takes on the same role as keyboards and input view controllers
     238// in UIKit. As such, we allow keyboard presentation and dismissal callbacks to work in extra zoom mode as well.
     239- (void)_webView:(WKWebView *)webView didPresentFocusedElementViewController:(UIViewController *)controller
     240{
     241    [self _invokeShowKeyboardCallbackIfNecessary];
     242}
     243
     244- (void)_webView:(WKWebView *)webView didDismissFocusedElementViewController:(UIViewController *)controller
     245{
     246    [self _invokeHideKeyboardCallbackIfNecessary];
     247}
     248
     249#endif // PLATFORM(IOS)
    235250
    236251@end
Note: See TracChangeset for help on using the changeset viewer.