⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 248341 in webkit


Ignore:
Timestamp:
Aug 6, 2019, 11:52:05 PM (7 years ago)
Author:
Kocsen Chung
Message:

Cherry-pick r247651. rdar://problem/54017866

[iOS] Entering 2FA code on idmsa.apple.com causes unexpected scrolling
​https://bugs.webkit.org/show_bug.cgi?id=199949
<rdar://problem/49944428>

Reviewed by Tim Horton and Megan Gardner.

Source/WebKit:

Since at least iOS 11, -[UIScrollView _adjustForAutomaticKeyboardInfo:animated:lastAdjustment:] adjusts the
scroll view's content offset to account for updated keyboard bottom insets. In WebKit, we call this method
whenever keyboard geometry changes (based on system notifications, such as UIKeyboardWillHideNotification).

When switching between focused form fields, we hide the keyboard for the previous focused element prior to
showing the keyboard for the newly focused element. This means that we will actually dismiss the keyboard in the
process of changing the focused element, which posts keyboard geometry notifications, which causes us to scroll
WKScrollView.

On iOS 12, this would be immediately followed by re-presenting the keyboard for the new focused element, which
causes us to adjust the scroll view back to its original position right away; this means that the scrolling that
happens as a result of adjusting for the keyboard insets after dismissal doesn't result in any visible change.

However, on iOS 13, after r239441 and r244546, we now defer scrolling and zooming to reveal the focused element
until later; this means the scrolling that happens as a result of initially dismissing the keyboard now causes a
consistent jump in the scroll view's scroll position (whereas on iOS 12, this only happens rarely, and the jump
is also less noticeable).

To mitigate this, we detect the case where we're moving focus from one element to another; if we're about to
show a keyboard for the newly focused element, then we should avoid scrolling as a result of the impending
"keyboard will hide" notification.

Test: fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html

  • UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]): (-[WKWebView _keyboardWillHide:]):
  • UIProcess/ios/WKContentViewInteraction.h:
  • UIProcess/ios/WKContentViewInteraction.mm: (shouldShowKeyboardForElement):

Add a helper to determine whether we're focusing an element which presents a "keyboard" (i.e. a UIKit input
view, as opposed to modal select pickers, modal date pickers, or fields with inputmode="none", for which we
don't show an input view).

(-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
(-[WKContentView shouldIgnoreKeyboardWillHideNotification]):

LayoutTests:

Add a new layout test to verify that moving focus between horizontally adjacent form controls doesn't induce
vertical scrolling.

  • fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields-expected.txt: Added.
  • fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html: Added.

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

Location:
branches/safari-608.1-branch
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608.1-branch/LayoutTests/ChangeLog

    r248131 r248341  
     12019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r247651. rdar://problem/54017866
     4
     5    [iOS] Entering 2FA code on idmsa.apple.com causes unexpected scrolling
     6    https://bugs.webkit.org/show_bug.cgi?id=199949
     7    <rdar://problem/49944428>
     8   
     9    Reviewed by Tim Horton and Megan Gardner.
     10   
     11    Source/WebKit:
     12   
     13    Since at least iOS 11, -[UIScrollView _adjustForAutomaticKeyboardInfo:animated:lastAdjustment:] adjusts the
     14    scroll view's content offset to account for updated keyboard bottom insets. In WebKit, we call this method
     15    whenever keyboard geometry changes (based on system notifications, such as UIKeyboardWillHideNotification).
     16   
     17    When switching between focused form fields, we hide the keyboard for the previous focused element prior to
     18    showing the keyboard for the newly focused element. This means that we will actually dismiss the keyboard in the
     19    process of changing the focused element, which posts keyboard geometry notifications, which causes us to scroll
     20    WKScrollView.
     21   
     22    On iOS 12, this would be immediately followed by re-presenting the keyboard for the new focused element, which
     23    causes us to adjust the scroll view back to its original position right away; this means that the scrolling that
     24    happens as a result of adjusting for the keyboard insets after dismissal doesn't result in any visible change.
     25   
     26    However, on iOS 13, after r239441 and r244546, we now defer scrolling and zooming to reveal the focused element
     27    until later; this means the scrolling that happens as a result of initially dismissing the keyboard now causes a
     28    consistent jump in the scroll view's scroll position (whereas on iOS 12, this only happens rarely, and the jump
     29    is also less noticeable).
     30   
     31    To mitigate this, we detect the case where we're moving focus from one element to another; if we're about to
     32    show a keyboard for the newly focused element, then we should avoid scrolling as a result of the impending
     33    "keyboard will hide" notification.
     34   
     35    Test: fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html
     36   
     37    * UIProcess/API/Cocoa/WKWebView.mm:
     38    (-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):
     39    (-[WKWebView _keyboardWillHide:]):
     40    * UIProcess/ios/WKContentViewInteraction.h:
     41    * UIProcess/ios/WKContentViewInteraction.mm:
     42    (shouldShowKeyboardForElement):
     43   
     44    Add a helper to determine whether we're focusing an element which presents a "keyboard" (i.e. a UIKit input
     45    view, as opposed to modal select pickers, modal date pickers, or fields with inputmode="none", for which we
     46    don't show an input view).
     47   
     48    (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
     49    (-[WKContentView shouldIgnoreKeyboardWillHideNotification]):
     50   
     51    LayoutTests:
     52   
     53    Add a new layout test to verify that moving focus between horizontally adjacent form controls doesn't induce
     54    vertical scrolling.
     55   
     56    * fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields-expected.txt: Added.
     57    * fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html: Added.
     58   
     59   
     60    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247651 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     61
     62    2019-07-19  Wenson Hsieh  <wenson_hsieh@apple.com>
     63
     64            [iOS] Entering 2FA code on idmsa.apple.com causes unexpected scrolling
     65            https://bugs.webkit.org/show_bug.cgi?id=199949
     66            <rdar://problem/49944428>
     67
     68            Reviewed by Tim Horton and Megan Gardner.
     69
     70            Add a new layout test to verify that moving focus between horizontally adjacent form controls doesn't induce
     71            vertical scrolling.
     72
     73            * fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields-expected.txt: Added.
     74            * fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html: Added.
     75
    1762019-08-01  Ryan Haddad  <ryanhaddad@apple.com>
    277

  • branches/safari-608.1-branch/Source/WebKit/ChangeLog

    r248340 r248341  
     12019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r247651. rdar://problem/54017866
     4
     5    [iOS] Entering 2FA code on idmsa.apple.com causes unexpected scrolling
     6    https://bugs.webkit.org/show_bug.cgi?id=199949
     7    <rdar://problem/49944428>
     8   
     9    Reviewed by Tim Horton and Megan Gardner.
     10   
     11    Source/WebKit:
     12   
     13    Since at least iOS 11, -[UIScrollView _adjustForAutomaticKeyboardInfo:animated:lastAdjustment:] adjusts the
     14    scroll view's content offset to account for updated keyboard bottom insets. In WebKit, we call this method
     15    whenever keyboard geometry changes (based on system notifications, such as UIKeyboardWillHideNotification).
     16   
     17    When switching between focused form fields, we hide the keyboard for the previous focused element prior to
     18    showing the keyboard for the newly focused element. This means that we will actually dismiss the keyboard in the
     19    process of changing the focused element, which posts keyboard geometry notifications, which causes us to scroll
     20    WKScrollView.
     21   
     22    On iOS 12, this would be immediately followed by re-presenting the keyboard for the new focused element, which
     23    causes us to adjust the scroll view back to its original position right away; this means that the scrolling that
     24    happens as a result of adjusting for the keyboard insets after dismissal doesn't result in any visible change.
     25   
     26    However, on iOS 13, after r239441 and r244546, we now defer scrolling and zooming to reveal the focused element
     27    until later; this means the scrolling that happens as a result of initially dismissing the keyboard now causes a
     28    consistent jump in the scroll view's scroll position (whereas on iOS 12, this only happens rarely, and the jump
     29    is also less noticeable).
     30   
     31    To mitigate this, we detect the case where we're moving focus from one element to another; if we're about to
     32    show a keyboard for the newly focused element, then we should avoid scrolling as a result of the impending
     33    "keyboard will hide" notification.
     34   
     35    Test: fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html
     36   
     37    * UIProcess/API/Cocoa/WKWebView.mm:
     38    (-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):
     39    (-[WKWebView _keyboardWillHide:]):
     40    * UIProcess/ios/WKContentViewInteraction.h:
     41    * UIProcess/ios/WKContentViewInteraction.mm:
     42    (shouldShowKeyboardForElement):
     43   
     44    Add a helper to determine whether we're focusing an element which presents a "keyboard" (i.e. a UIKit input
     45    view, as opposed to modal select pickers, modal date pickers, or fields with inputmode="none", for which we
     46    don't show an input view).
     47   
     48    (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
     49    (-[WKContentView shouldIgnoreKeyboardWillHideNotification]):
     50   
     51    LayoutTests:
     52   
     53    Add a new layout test to verify that moving focus between horizontally adjacent form controls doesn't induce
     54    vertical scrolling.
     55   
     56    * fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields-expected.txt: Added.
     57    * fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html: Added.
     58   
     59   
     60    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247651 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     61
     62    2019-07-19  Wenson Hsieh  <wenson_hsieh@apple.com>
     63
     64            [iOS] Entering 2FA code on idmsa.apple.com causes unexpected scrolling
     65            https://bugs.webkit.org/show_bug.cgi?id=199949
     66            <rdar://problem/49944428>
     67
     68            Reviewed by Tim Horton and Megan Gardner.
     69
     70            Since at least iOS 11, -[UIScrollView _adjustForAutomaticKeyboardInfo:animated:lastAdjustment:] adjusts the
     71            scroll view's content offset to account for updated keyboard bottom insets. In WebKit, we call this method
     72            whenever keyboard geometry changes (based on system notifications, such as UIKeyboardWillHideNotification).
     73
     74            When switching between focused form fields, we hide the keyboard for the previous focused element prior to
     75            showing the keyboard for the newly focused element. This means that we will actually dismiss the keyboard in the
     76            process of changing the focused element, which posts keyboard geometry notifications, which causes us to scroll
     77            WKScrollView.
     78
     79            On iOS 12, this would be immediately followed by re-presenting the keyboard for the new focused element, which
     80            causes us to adjust the scroll view back to its original position right away; this means that the scrolling that
     81            happens as a result of adjusting for the keyboard insets after dismissal doesn't result in any visible change.
     82
     83            However, on iOS 13, after r239441 and r244546, we now defer scrolling and zooming to reveal the focused element
     84            until later; this means the scrolling that happens as a result of initially dismissing the keyboard now causes a
     85            consistent jump in the scroll view's scroll position (whereas on iOS 12, this only happens rarely, and the jump
     86            is also less noticeable).
     87
     88            To mitigate this, we detect the case where we're moving focus from one element to another; if we're about to
     89            show a keyboard for the newly focused element, then we should avoid scrolling as a result of the impending
     90            "keyboard will hide" notification.
     91
     92            Test: fast/forms/ios/no-scrolling-when-moving-focus-between-adjacent-fields.html
     93
     94            * UIProcess/API/Cocoa/WKWebView.mm:
     95            (-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):
     96            (-[WKWebView _keyboardWillHide:]):
     97            * UIProcess/ios/WKContentViewInteraction.h:
     98            * UIProcess/ios/WKContentViewInteraction.mm:
     99            (shouldShowKeyboardForElement):
     100
     101            Add a helper to determine whether we're focusing an element which presents a "keyboard" (i.e. a UIKit input
     102            view, as opposed to modal select pickers, modal date pickers, or fields with inputmode="none", for which we
     103            don't show an input view).
     104
     105            (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
     106            (-[WKContentView shouldIgnoreKeyboardWillHideNotification]):
     107
    11082019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
    2109

  • branches/safari-608.1-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

    r248298 r248341  
    33313331        [_scrollView _adjustForAutomaticKeyboardInfo:keyboardInfo animated:YES lastAdjustment:&_lastAdjustmentForScroller];
    33323332        CGFloat bottomInsetAfterAdjustment = [_scrollView contentInset].bottom;
     3333        // FIXME: This "total bottom content inset adjustment" mechanism hasn't worked since iOS 11, since -_adjustForAutomaticKeyboardInfo:animated:lastAdjustment:
     3334        // no longer sets -[UIScrollView contentInset] for apps linked on or after iOS 11. We should consider removing this logic, since the original bug this was
     3335        // intended to fix, <rdar://problem/23202254>, remains fixed through other means.
    33333336        if (bottomInsetBeforeAdjustment != bottomInsetAfterAdjustment)
    33343337            _totalScrollViewBottomInsetAdjustmentForKeyboard += bottomInsetAfterAdjustment - bottomInsetBeforeAdjustment;
    … …  
    33733376- (void)_keyboardWillHide:(NSNotification *)notification
    33743377{
    3375     // Ignore keyboard will hide notifications sent during rotation. They're just there for
    3376     // backwards compatibility reasons and processing the will hide notification would
    3377     // temporarily screw up the unobscured view area.
    3378     if ([[UIPeripheralHost sharedInstance] rotationState])
     3378    if ([_contentView shouldIgnoreKeyboardWillHideNotification])
    33793379        return;
    33803380

  • branches/safari-608.1-branch/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

    r247798 r248341  
    346346    BOOL _needsDeferredEndScrollingSelectionUpdate;
    347347    BOOL _isChangingFocus;
     348    BOOL _isFocusingElementWithKeyboard;
    348349    BOOL _isBlurringFocusedElement;
    349350
    … …  
    398399@property (nonatomic, readonly) BOOL isEditable;
    399400@property (nonatomic, readonly) BOOL shouldHideSelectionWhenScrolling;
     401@property (nonatomic, readonly) BOOL shouldIgnoreKeyboardWillHideNotification;
    400402@property (nonatomic, readonly) const WebKit::InteractionInformationAtPosition& positionInformation;
    401403@property (nonatomic, readonly) const WebKit::WKAutoCorrectionData& autocorrectionData;

  • branches/safari-608.1-branch/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

    r248299 r248341  
    51615161}
    51625162
     5163static bool shouldShowKeyboardForElement(const WebKit::FocusedElementInformation& information)
     5164{
     5165    if (information.inputMode == WebCore::InputMode::None)
     5166        return false;
     5167
     5168    if (information.elementType == WebKit::InputType::Drawing)
     5169        return false;
     5170
     5171    if (mayContainSelectableText(information.elementType))
     5172        return true;
     5173
     5174    return !currentUserInterfaceIdiomIsPad();
     5175}
     5176
    51635177static WebCore::FloatRect rectToRevealWhenZoomingToFocusedElement(const WebKit::FocusedElementInformation& elementInfo, const WebKit::EditorState& editorState)
    51645178{
    … …  
    52075221{
    52085222    SetForScope<BOOL> isChangingFocusForScope { _isChangingFocus, hasFocusedElement(_focusedElementInformation) };
     5223    SetForScope<BOOL> isFocusingElementWithKeyboardForScope { _isFocusingElementWithKeyboard, shouldShowKeyboardForElement(information) };
     5224
    52095225    auto inputViewUpdateDeferrer = std::exchange(_inputViewUpdateDeferrer, nullptr);
    52105226
    … …  
    54065422    if (!_isChangingFocus)
    54075423        _didAccessoryTabInitiateFocus = NO;
     5424}
     5425
     5426- (BOOL)shouldIgnoreKeyboardWillHideNotification
     5427{
     5428    // Ignore keyboard will hide notifications sent during rotation. They're just there for
     5429    // backwards compatibility reasons and processing the will hide notification would
     5430    // temporarily screw up the unobscured view area.
     5431    if (UIPeripheralHost.sharedInstance.rotationState)
     5432        return YES;
     5433
     5434    if (_isChangingFocus && _isFocusingElementWithKeyboard)
     5435        return YES;
     5436
     5437    return NO;
    54085438}
    54095439
Note: See TracChangeset for help on using the changeset viewer.