Changeset 248338 in webkit


Ignore:
Timestamp:
Aug 6, 2019 8:15:19 PM (5 years ago)
Author:
rniwa@webkit.org
Message:

[iPadOS] slides.google.com: tapping near cursor in a slide title focuses the speaker notes
​https://bugs.webkit.org/show_bug.cgi?id=200216

Reviewed by Wenson Hsieh.

Source/WebKit:

The bug was caused by a race condition between Google slides removing inputmode="none" from the hidden
content editable and updating the focused region upon receiving a pointerup event, which happens after
the Google slides had already updated its page layout & coordinates based on new visual viewport with
the software keyboard's boudning rect taken into account.

Delay bringing up the software keyboard for a inputmode change until all touches are released.

In the future, we could consider also delaying the software keyboard to be brought in general until
touchend / pointerup events are dispatched but this is rather risky since that could affact random
other websites while Google suites is the only major site to make use of inputmode="none".

This patch also reverts r243044, which was added for Google slides, since it's no longer needed and
interferes with this patch by adding another way to bring up the software keyboard.

Note: Adjusting touchend / pointerup coordinates while the keyboard is being brought up doesn't work
because the page had already updated the layout by then based on new visual viewport size.

Test: fast/forms/ios/inputmode-change-update-keyboard-after-pointerup.html

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::handleTouchEventSynchronously): Call didReleaseAllTouchPoints when all
touches are released.
(WebKit::WebPageProxy::handleTouchEventAsynchronously): Ditto.
(WebKit::WebPageProxy::handleTouchEvent): Ditto.

  • UIProcess/WebPageProxy.h:

(WebKit::WebPageProxy::didReleaseAllTouchPoints): Added for non-iOS platforms.
(WebKit::WebPageProxy::m_pendingInputModeChange): Added. Used when inputmode is changed while
there is an on-going touch interaction.

  • UIProcess/ios/WebPageProxyIOS.mm:

(WebKit::WebPageProxy::elementDidFocus): Clear m_pendingInputModeChange when a new element is focused.
(WebKit::WebPageProxy::elementDidBlur): Ditto for bluring.
(WebKit::WebPageProxy::focusedElementDidChangeInputMode): Don't bring up the software keyboard now if
there are on-going touches by exiting early after setting m_pendingInputModeChange.
(WebKit::WebPageProxy::didReleaseAllTouchPoints): Bring up the software keyboard if inputmode
had changed from "none" to something else.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::dispatchTouchEvent): Removed the code added by r243044.

LayoutTests:

Added a new regression test and removed the one added for r243044.

  • fast/events/touch/ios/show-keyboard-after-preventing-touchstart-expected.txt: Removed.
  • fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html: Removed.
  • fast/forms/ios/inputmode-change-update-keyboard-after-pointerup-expected.txt: Added.
  • fast/forms/ios/inputmode-change-update-keyboard-after-pointerup.html: Added.
  • fast/forms/ios/inputmode-change-update-keyboard.html: Fixed the test for manual testing.
Location:
trunk
Files:
2 added
2 deleted
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r248327 r248338  
     12019-08-06  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        [iPadOS] slides.google.com: tapping near cursor in a slide title focuses the speaker notes
     4        https://bugs.webkit.org/show_bug.cgi?id=200216
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        Added a new regression test and removed the one added for r243044.
     9
     10        * fast/events/touch/ios/show-keyboard-after-preventing-touchstart-expected.txt: Removed.
     11        * fast/events/touch/ios/show-keyboard-after-preventing-touchstart.html: Removed.
     12        * fast/forms/ios/inputmode-change-update-keyboard-after-pointerup-expected.txt: Added.
     13        * fast/forms/ios/inputmode-change-update-keyboard-after-pointerup.html: Added.
     14        * fast/forms/ios/inputmode-change-update-keyboard.html: Fixed the test for manual testing.
     15
    1162019-08-06  Commit Queue  <commit-queue@webkit.org>
    217

  • trunk/LayoutTests/fast/forms/ios/inputmode-change-update-keyboard.html

    r244397 r248338  
    3131
    3232async function runTest() {
    33     await UIHelper.setHardwareKeyboardAttached(false);
     33    if (window.testRunner)
     34        await UIHelper.setHardwareKeyboardAttached(false);
    3435
    3536    debug('inputmode="text"');

  • trunk/Source/WebKit/ChangeLog

    r248336 r248338  
     12019-08-06  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        [iPadOS] slides.google.com: tapping near cursor in a slide title focuses the speaker notes
     4        https://bugs.webkit.org/show_bug.cgi?id=200216
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        The bug was caused by a race condition between Google slides removing inputmode="none" from the hidden
     9        content editable and updating the focused region upon receiving a pointerup event, which happens after
     10        the Google slides had already updated its page layout & coordinates based on new visual viewport with
     11        the software keyboard's boudning rect taken into account.
     12
     13        Delay bringing up the software keyboard for a inputmode change until all touches are released.
     14
     15        In the future, we could consider also delaying the software keyboard to be brought in general until
     16        touchend / pointerup events are dispatched but this is rather risky since that could affact random
     17        other websites while Google suites is the only major site to make use of inputmode="none".
     18
     19        This patch also reverts r243044, which was added for Google slides, since it's no longer needed and
     20        interferes with this patch by adding another way to bring up the software keyboard.
     21
     22        Note: Adjusting touchend / pointerup coordinates while the keyboard is being brought up doesn't work
     23        because the page had already updated the layout by then based on new visual viewport size.
     24
     25        Test: fast/forms/ios/inputmode-change-update-keyboard-after-pointerup.html
     26
     27        * UIProcess/WebPageProxy.cpp:
     28        (WebKit::WebPageProxy::handleTouchEventSynchronously): Call didReleaseAllTouchPoints when all
     29        touches are released.
     30        (WebKit::WebPageProxy::handleTouchEventAsynchronously): Ditto.
     31        (WebKit::WebPageProxy::handleTouchEvent): Ditto.
     32        * UIProcess/WebPageProxy.h:
     33        (WebKit::WebPageProxy::didReleaseAllTouchPoints): Added for non-iOS platforms.
     34        (WebKit::WebPageProxy::m_pendingInputModeChange): Added. Used when inputmode is changed while
     35        there is an on-going touch interaction.
     36        * UIProcess/ios/WebPageProxyIOS.mm:
     37        (WebKit::WebPageProxy::elementDidFocus): Clear m_pendingInputModeChange when a new element is focused.
     38        (WebKit::WebPageProxy::elementDidBlur): Ditto for bluring.
     39        (WebKit::WebPageProxy::focusedElementDidChangeInputMode): Don't bring up the software keyboard now if
     40        there are on-going touches by exiting early after setting m_pendingInputModeChange.
     41        (WebKit::WebPageProxy::didReleaseAllTouchPoints): Bring up the software keyboard if inputmode
     42        had changed from "none" to something else.
     43        * WebProcess/WebPage/WebPage.cpp:
     44        (WebKit::WebPage::dispatchTouchEvent): Removed the code added by r243044.
     45
    1462019-08-06  Fujii Hironori  <Hironori.Fujii@sony.com>
    247

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

    r248336 r248338  
    26892689    m_process->responsivenessTimer().stop();
    26902690
    2691     if (event.allTouchPointsAreReleased())
     2691    if (event.allTouchPointsAreReleased()) {
    26922692        m_touchAndPointerEventTracking.reset();
     2693        didReleaseAllTouchPoints();
     2694    }
    26932695}
    26942696
    … …  
    27122714    m_process->send(Messages::EventDispatcher::TouchEvent(m_pageID, event), 0);
    27132715
    2714     if (event.allTouchPointsAreReleased())
     2716    if (event.allTouchPointsAreReleased()) {
    27152717        m_touchAndPointerEventTracking.reset();
     2718        didReleaseAllTouchPoints();
     2719    }
    27162720}
    27172721
    … …  
    27462750    }
    27472751
    2748     if (event.allTouchPointsAreReleased())
     2752    if (event.allTouchPointsAreReleased()) {
    27492753        m_touchAndPointerEventTracking.reset();
     2754        didReleaseAllTouchPoints();
     2755    }
    27502756}
    27512757#endif // ENABLE(TOUCH_EVENTS)

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

    r248029 r248338  
    19471947    void elementDidBlur();
    19481948    void focusedElementDidChangeInputMode(WebCore::InputMode);
     1949    void didReleaseAllTouchPoints();
    19491950    void didReceiveEditorStateUpdateAfterFocus();
    19501951
    … …  
    19551956    void disableInspectorNodeSearch();
    19561957    void focusedElementInformationCallback(const FocusedElementInformation&, CallbackID);
     1958#else
     1959    void didReleaseAllTouchPoints() { }
    19571960#endif // PLATFORM(IOS_FAMILY)
    19581961
    … …  
    21752178
    21762179#if PLATFORM(IOS_FAMILY)
     2180    Optional<WebCore::InputMode> m_pendingInputModeChange;
    21772181    Optional<WebCore::ViewportArguments> m_overrideViewportArguments;
    21782182    VisibleContentRectUpdateInfo m_lastVisibleContentRectUpdate;

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

    r248129 r248338  
    906906void WebPageProxy::elementDidFocus(const FocusedElementInformation& information, bool userIsInteracting, bool blurPreviousNode, OptionSet<WebCore::ActivityState::Flag> activityStateChanges, const UserData& userData)
    907907{
     908    m_pendingInputModeChange = WTF::nullopt;
    908909    m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = true;
    909910
    … …  
    920921void WebPageProxy::elementDidBlur()
    921922{
     923    m_pendingInputModeChange = WTF::nullopt;
    922924    m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false;
    923925    m_deferredElementDidFocusArguments = nullptr;
    … …  
    927929void WebPageProxy::focusedElementDidChangeInputMode(WebCore::InputMode mode)
    928930{
     931#if ENABLE(TOUCH_EVENTS)
     932    if (m_touchAndPointerEventTracking.isTrackingAnything()) {
     933        m_pendingInputModeChange = mode;
     934        return;
     935    }
     936#endif
     937
    929938    pageClient().focusedElementDidChangeInputMode(mode);
     939}
     940
     941void WebPageProxy::didReleaseAllTouchPoints()
     942{
     943    if (!m_pendingInputModeChange)
     944        return;
     945
     946    pageClient().focusedElementDidChangeInputMode(*m_pendingInputModeChange);
     947    m_pendingInputModeChange = WTF::nullopt;
    930948}
    931949

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r248203 r248338  
    28982898{
    28992899    SetForScope<bool> userIsInteractingChange { m_userIsInteracting, true };
    2900 
    2901     auto oldFocusedFrame = makeRefPtr(m_page->focusController().focusedFrame());
    2902     auto oldFocusedElement = makeRefPtr(oldFocusedFrame ? oldFocusedFrame->document()->focusedElement() : nullptr);
    2903 
    29042900    m_lastInteractionLocation = touchEvent.position();
    29052901    CurrentEvent currentEvent(touchEvent);
    29062902    handled = handleTouchEvent(touchEvent, m_page.get());
    29072903    updatePotentialTapSecurityOrigin(touchEvent, handled);
    2908 
    2909     if (handled && oldFocusedElement) {
    2910         auto newFocusedFrame = makeRefPtr(m_page->focusController().focusedFrame());
    2911         auto newFocusedElement = makeRefPtr(newFocusedFrame ? newFocusedFrame->document()->focusedElement() : nullptr);
    2912         if (oldFocusedElement == newFocusedElement && isTransparentOrFullyClipped(*newFocusedElement))
    2913             elementDidRefocus(*newFocusedElement);
    2914     }
    29152904}
    29162905
Note: See TracChangeset for help on using the changeset viewer.