Changeset 278515 in webkit


Ignore:
Timestamp:
Jun 4, 2021, 5:57:43 PM (4 years ago)
Author:
Wenson Hsieh
Message:

[iOS] Meaningful click heuristic should account for media state changes
​https://bugs.webkit.org/show_bug.cgi?id=226655
rdar://78330664

Reviewed by Tim Horton and Devin Rousso.

Source/WebKit:

Teach the "meaningful click" heuristic about changes to media element state flags. See comments below for more
details.

Test: fast/events/ios/meaningful-click-when-playing-media.html

  • WebProcess/WebCoreSupport/WebChromeClient.cpp:

(WebKit::WebChromeClient::isPlayingMediaDidChange):

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::isPlayingMediaDidChange):

Refactor some logic here so that the WebChromeClient calls into WebPage, which then sends an IPC message to the
UI process and additionally calls into a private method for platform-specific logic (see WebPageIOS.mm).

  • WebProcess/WebPage/WebPage.h:

Replace m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick with another flag,
m_currentSyntheticClickMayNotBeMeaningful, that is initially set to true at the beginning of
WebPage::completeSyntheticClick, and consulted after the events have been dispatched to see if anything has
set it to false (currently, this includes only playing media state changes and handled click events by the
page). If the flag is set to false, we then consider the click to have been "meaningful", with respect to the
-_webView:didNotHandleTapAsMeaningfulClickAtPoint: UI delegate method.

(WebKit::WebPage::platformIsPlayingMediaDidChange):

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::completeSyntheticClick):
(WebKit::WebPage::didHandleOrPreventMouseDownOrMouseUpEvent):
(WebKit::WebPage::platformIsPlayingMediaDidChange):

Make these set m_currentSyntheticClickMayNotBeMeaningful to false.

LayoutTests:

Add a layout test to verify that tapping to play or pause a video triggers "meaningful" synthetic clicks.

  • fast/events/ios/meaningful-click-when-playing-media-expected.txt: Added.
  • fast/events/ios/meaningful-click-when-playing-media.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r278512 r278515  
     12021-06-04  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Meaningful click heuristic should account for media state changes
     4        https://bugs.webkit.org/show_bug.cgi?id=226655
     5        rdar://78330664
     6
     7        Reviewed by Tim Horton and Devin Rousso.
     8
     9        Add a layout test to verify that tapping to play or pause a video triggers "meaningful" synthetic clicks.
     10
     11        * fast/events/ios/meaningful-click-when-playing-media-expected.txt: Added.
     12        * fast/events/ios/meaningful-click-when-playing-media.html: Added.
     13
    1142021-06-04  Devin Rousso  <drousso@apple.com>
    215

  • trunk/Source/WebKit/ChangeLog

    r278514 r278515  
     12021-06-04  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Meaningful click heuristic should account for media state changes
     4        https://bugs.webkit.org/show_bug.cgi?id=226655
     5        rdar://78330664
     6
     7        Reviewed by Tim Horton and Devin Rousso.
     8
     9        Teach the "meaningful click" heuristic about changes to media element state flags. See comments below for more
     10        details.
     11
     12        Test: fast/events/ios/meaningful-click-when-playing-media.html
     13
     14        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
     15        (WebKit::WebChromeClient::isPlayingMediaDidChange):
     16        * WebProcess/WebPage/WebPage.cpp:
     17        (WebKit::WebPage::isPlayingMediaDidChange):
     18
     19        Refactor some logic here so that the WebChromeClient calls into WebPage, which then sends an IPC message to the
     20        UI process and additionally calls into a private method for platform-specific logic (see WebPageIOS.mm).
     21
     22        * WebProcess/WebPage/WebPage.h:
     23
     24        Replace `m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick` with another flag,
     25        `m_currentSyntheticClickMayNotBeMeaningful`, that is initially set to `true` at the beginning of
     26        `WebPage::completeSyntheticClick`, and consulted after the events have been dispatched to see if anything has
     27        set it to `false` (currently, this includes only playing media state changes and handled click events by the
     28        page). If the flag is set to `false`, we then consider the click to have been "meaningful", with respect to the
     29        `-_webView:didNotHandleTapAsMeaningfulClickAtPoint:` UI delegate method.
     30
     31        (WebKit::WebPage::platformIsPlayingMediaDidChange):
     32        * WebProcess/WebPage/ios/WebPageIOS.mm:
     33        (WebKit::WebPage::completeSyntheticClick):
     34        (WebKit::WebPage::didHandleOrPreventMouseDownOrMouseUpEvent):
     35        (WebKit::WebPage::platformIsPlayingMediaDidChange):
     36
     37        Make these set `m_currentSyntheticClickMayNotBeMeaningful` to `false`.
     38
    1392021-06-04  Ryosuke Niwa  <rniwa@webkit.org>
    240

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp

    r278482 r278515  
    12361236void WebChromeClient::isPlayingMediaDidChange(MediaProducer::MediaStateFlags state)
    12371237{
    1238     m_page.send(Messages::WebPageProxy::IsPlayingMediaDidChange(state));
     1238    m_page.isPlayingMediaDidChange(state);
    12391239}
    12401240

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

    r278394 r278515  
    73617361}
    73627362
     7363void WebPage::isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags state)
     7364{
     7365    platformIsPlayingMediaDidChange();
     7366    send(Messages::WebPageProxy::IsPlayingMediaDidChange(state));
     7367}
    73637368
    73647369#if ENABLE(MEDIA_USAGE)

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

    r278394 r278515  
    14021402#endif
    14031403
     1404    void isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags);
     1405
    14041406#if ENABLE(IMAGE_EXTRACTION)
    14051407    void requestImageExtraction(WebCore::Element&, CompletionHandler<void(RefPtr<WebCore::Element>&&)>&&);
    … …  
    18981900    void consumeNetworkExtensionSandboxExtensions(const SandboxExtension::HandleArray&);
    18991901
     1902    void platformIsPlayingMediaDidChange();
     1903
    19001904    WebCore::PageIdentifier m_identifier;
    19011905
    … …  
    21862190    WebCore::FloatPoint m_potentialTapLocation;
    21872191    RefPtr<WebCore::SecurityOrigin> m_potentialTapSecurityOrigin;
    2188     bool m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick { false };
    2189 
     2192
     2193    bool m_currentSyntheticClickMayNotBeMeaningful { true };
    21902194    bool m_hasReceivedVisibleContentRectsAfterDidCommitLoad { false };
    21912195    bool m_hasRestoredExposedContentRectAfterDidCommitLoad { false };
    … …  
    23592363inline void WebPage::didHandleOrPreventMouseDownOrMouseUpEvent() { }
    23602364inline void WebPage::prepareToRunModalJavaScriptDialog() { }
     2365inline void WebPage::platformIsPlayingMediaDidChange() { }
    23612366#endif
    23622367

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r278459 r278515  
    875875    bool metaKey = modifiers.contains(WebEvent::Modifier::MetaKey);
    876876
    877     m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick = false;
     877    m_currentSyntheticClickMayNotBeMeaningful = true;
    878878
    879879    tapWasHandled |= mainframe.eventHandler().handleMousePressEvent(PlatformMouseEvent(roundedAdjustedPoint, roundedAdjustedPoint, LeftButton, PlatformEvent::MousePressed, 1, shiftKey, ctrlKey, altKey, metaKey, WallTime::now(), WebCore::ForceAtClick, syntheticClickType, pointerId));
    … …  
    904904
    905905    bool shouldDispatchDidNotHandleTapAsMeaningfulClickAtPoint = ([&] {
    906         if (m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick)
     906        if (!m_currentSyntheticClickMayNotBeMeaningful)
    907907            return false;
    908908
    … …  
    925925        send(Messages::WebPageProxy::DidNotHandleTapAsClick(roundedIntPoint(location)));
    926926
    927     m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick = false;
    928    
    929927    send(Messages::WebPageProxy::DidCompleteSyntheticClick());
    930928}
    … …  
    948946void WebPage::didHandleOrPreventMouseDownOrMouseUpEvent()
    949947{
    950     m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick = true;
     948    m_currentSyntheticClickMayNotBeMeaningful = false;
    951949}
    952950
    … …  
    46954693}
    46964694
     4695void WebPage::platformIsPlayingMediaDidChange()
     4696{
     4697    m_currentSyntheticClickMayNotBeMeaningful = false;
     4698}
     4699
    46974700} // namespace WebKit
    46984701
Note: See TracChangeset for help on using the changeset viewer.