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

Changeset 280374 in webkit


Ignore:
Timestamp:
Jul 27, 2021, 8:56:44 PM (5 years ago)
Author:
Devin Rousso
Message:

[Modern Media Controls] [macOS] Overflow button still shows as on even after contextmenu is dismissed
​https://bugs.webkit.org/show_bug.cgi?id=228310
<rdar://problem/81124786>

Reviewed by Wenson Hsieh.

Source/WebCore:

Test: media/modern-media-controls/overflow-support/button-state.html

  • page/ContextMenuController.h:
  • page/ContextMenuController.cpp:

(WebCore::ContextMenuController::didDismissContextMenu): Added.

  • page/ContextMenuProvider.h:

(WebCore::ContextMenuProvider::didDismissContextMenu): Added.
Add plumbing from WebKit to the ContextMenuProvider for when the contextmenu is dismissed.

  • Modules/mediacontrols/MediaControlsHost.cpp:

(WebCore::MediaControlsContextMenuProvider::didDismissContextMenu): Added.
When the contextmenu is dismissed, invoke the callback provided to MediaControlsHost::showMediaControlsContextMenu
as that's the signal to the MediaController that the contextmenu interaction is over,
which adjusts the on state of the Button.

Source/WebKit:

Pipe along the -[NSMenuDelegate menuDidClose:] signal to the WebProcess so the WebCore::ContextMenuProvider
can handle it if needed.

  • UIProcess/WebContextMenuProxy.cpp:

(WebKit::WebContextMenuProxy::useContextMenuItems):

  • UIProcess/mac/WebContextMenuProxyMac.mm:

(-[WKMenuDelegate menuWillOpen:]):
(-[WKMenuDelegate menuDidClose:]):

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::showContextMenu):
(WebKit::WebPageProxy::didShowContextMenu):
(WebKit::WebPageProxy::didDismissContextMenu): Added.

  • WebProcess/WebPage/WebContextMenu.cpp:

(WebKit::WebContextMenu::show):

  • WebProcess/WebPage/WebPage.messages.in:
  • WebProcess/WebPage/WebPage.h:

(WebKit::WebPage::startWaitingForContextMenuToShow): Added.
(WebKit::WebPage::contextMenuShowing): Deleted.
(WebKit::WebPage::contextMenuHidden): Deleted.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::didShowContextMenu): Added.
(WebKit::WebPage::didDismissContextMenu): Added.
(WebKit::WebPage::mouseEvent):
Drive-by: Rename m_contextMenuShowing to m_waitingForContextMenuToShow for clarity.

Tools:

  • WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:

(-[TestRunnerWKWebView dismissActiveMenu]):
(-[TestRunnerWKWebView immediatelyDismissContextMenuIfNeeded]):
(-[TestRunnerWKWebView _dismissAllContextMenuInteractions]): Added.

LayoutTests:

  • media/modern-media-controls/overflow-support/button-state.html: Added.
  • media/modern-media-controls/overflow-support/button-state-expected.txt: Added.
  • media/modern-media-controls/overflow-support/playback-speed.html:
  • media/modern-media-controls/tracks-support/click-track-in-contextmenu.html:

Adjust how these tests wait for and dismiss the contextmenu.

Location:
trunk
Files:
2 added
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r280368 r280374  
     12021-07-27  Devin Rousso  <drousso@apple.com>
     2
     3        [Modern Media Controls] [macOS] Overflow button still shows as `on` even after contextmenu is dismissed
     4        https://bugs.webkit.org/show_bug.cgi?id=228310
     5        <rdar://problem/81124786>
     6
     7        Reviewed by Wenson Hsieh.
     8
     9        * media/modern-media-controls/overflow-support/button-state.html: Added.
     10        * media/modern-media-controls/overflow-support/button-state-expected.txt: Added.
     11
     12        * media/modern-media-controls/overflow-support/playback-speed.html:
     13        * media/modern-media-controls/tracks-support/click-track-in-contextmenu.html:
     14        Adjust how these tests wait for and dismiss the contextmenu.
     15
    1162021-07-27  Amir Mark Jr  <amir_mark@apple.com>
    217

  • trunk/LayoutTests/media/modern-media-controls/overflow-support/playback-speed.html

    r278618 r280374  
    3131            shouldBe("checkedItems.length", "1");
    3232            shouldBeEqualToString("checkedItems[0].title", `1${multiplicationSign}`);
     33
     34            await new Promise((resolve, reject) => {
     35                testRunner.runUIScript(`
     36                    uiController.dismissMenu();
     37                    uiController.uiScriptComplete();
     38                `, resolve);
     39            });
    3340            await UIHelper.waitForContextMenuToHide();
    3441

  • trunk/LayoutTests/media/modern-media-controls/tracks-support/click-track-in-contextmenu.html

    r277616 r280374  
    4444{
    4545    debug("Selecting 'French' track...");
    46     await getTracksContextMenu();
     46    await UIHelper.waitForContextMenuToShow();
    4747    await UIHelper.chooseMenuAction("French Sound");
    4848}
    … …  
    5151{
    5252    debug("Selecting 'Spanish' track...");
    53     await getTracksContextMenu();
     53    await UIHelper.waitForContextMenuToShow();
    5454    await UIHelper.chooseMenuAction("Spanish Sound");
    5555}

  • trunk/Source/WebCore/ChangeLog

    r280369 r280374  
     12021-07-27  Devin Rousso  <drousso@apple.com>
     2
     3        [Modern Media Controls] [macOS] Overflow button still shows as `on` even after contextmenu is dismissed
     4        https://bugs.webkit.org/show_bug.cgi?id=228310
     5        <rdar://problem/81124786>
     6
     7        Reviewed by Wenson Hsieh.
     8
     9        Test: media/modern-media-controls/overflow-support/button-state.html
     10
     11        * page/ContextMenuController.h:
     12        * page/ContextMenuController.cpp:
     13        (WebCore::ContextMenuController::didDismissContextMenu): Added.
     14        * page/ContextMenuProvider.h:
     15        (WebCore::ContextMenuProvider::didDismissContextMenu): Added.
     16        Add plumbing from WebKit to the `ContextMenuProvider` for when the contextmenu is dismissed.
     17
     18        * Modules/mediacontrols/MediaControlsHost.cpp:
     19        (WebCore::MediaControlsContextMenuProvider::didDismissContextMenu): Added.
     20        When the contextmenu is dismissed, invoke the callback provided to `MediaControlsHost::showMediaControlsContextMenu`
     21        as that's the signal to the `MediaController` that the contextmenu interaction is over,
     22        which adjusts the `on` state of the `Button`.
     23
    1242021-07-27  Ryosuke Niwa  <rniwa@webkit.org>
    225

  • trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp

    r279914 r280374  
    389389    }
    390390
     391    void didDismissContextMenu() override
     392    {
     393        if (m_callback)
     394            m_callback(ContextMenuItemTagNoAction);
     395    }
     396
    391397    void contextMenuItemSelected(ContextMenuAction action, const String&) override
    392398    {
    … …  
    456462{
    457463#if USE(UICONTEXTMENU) || (ENABLE(CONTEXT_MENUS) && USE(ACCESSIBILITY_CONTEXT_MENUS))
    458     if (m_showMediaControlsContextMenuCallback) {
    459 #if USE(UICONTEXTMENU)
     464    if (m_showMediaControlsContextMenuCallback)
    460465        return false;
    461 #elif (ENABLE(CONTEXT_MENUS) && USE(ACCESSIBILITY_CONTEXT_MENUS))
    462         // FIXME: `contextMenuCleared` is invoked between show and item selected so we may have a pending callback.
    463         std::exchange(m_showMediaControlsContextMenuCallback, nullptr)->handleEvent();
    464 #endif
    465     }
    466466
    467467    m_showMediaControlsContextMenuCallback = WTFMove(callback);

  • trunk/Source/WebCore/page/ContextMenuController.cpp

    r279164 r280374  
    173173}
    174174
     175void ContextMenuController::didDismissContextMenu()
     176{
     177    if (m_menuProvider)
     178        m_menuProvider->didDismissContextMenu();
     179}
     180
    175181static void openNewWindow(const URL& urlToLoad, Frame& frame, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
    176182{

  • trunk/Source/WebCore/page/ContextMenuController.h

    r277295 r280374  
    5757
    5858    void populate();
     59    WEBCORE_EXPORT void didDismissContextMenu();
    5960    WEBCORE_EXPORT void contextMenuItemSelected(ContextMenuAction, const String& title);
    6061    void addInspectElementItem();

  • trunk/Source/WebCore/page/ContextMenuProvider.h

    r274521 r280374  
    4646
    4747    virtual void populateContextMenu(ContextMenu*) = 0;
     48    virtual void didDismissContextMenu() { }
    4849    virtual void contextMenuItemSelected(ContextMenuAction, const String& title) = 0;
    4950    virtual void contextMenuCleared() = 0;

  • trunk/Source/WebKit/ChangeLog

    r280361 r280374  
     12021-07-27  Devin Rousso  <drousso@apple.com>
     2
     3        [Modern Media Controls] [macOS] Overflow button still shows as `on` even after contextmenu is dismissed
     4        https://bugs.webkit.org/show_bug.cgi?id=228310
     5        <rdar://problem/81124786>
     6
     7        Reviewed by Wenson Hsieh.
     8
     9        Pipe along the `-[NSMenuDelegate menuDidClose:]` signal to the WebProcess so the `WebCore::ContextMenuProvider`
     10        can handle it if needed.
     11
     12        * UIProcess/WebContextMenuProxy.cpp:
     13        (WebKit::WebContextMenuProxy::useContextMenuItems):
     14        * UIProcess/mac/WebContextMenuProxyMac.mm:
     15        (-[WKMenuDelegate menuWillOpen:]):
     16        (-[WKMenuDelegate menuDidClose:]):
     17
     18        * UIProcess/WebPageProxy.h:
     19        * UIProcess/WebPageProxy.cpp:
     20        (WebKit::WebPageProxy::showContextMenu):
     21        (WebKit::WebPageProxy::didShowContextMenu):
     22        (WebKit::WebPageProxy::didDismissContextMenu): Added.
     23
     24        * WebProcess/WebPage/WebContextMenu.cpp:
     25        (WebKit::WebContextMenu::show):
     26
     27        * WebProcess/WebPage/WebPage.messages.in:
     28        * WebProcess/WebPage/WebPage.h:
     29        (WebKit::WebPage::startWaitingForContextMenuToShow): Added.
     30        (WebKit::WebPage::contextMenuShowing): Deleted.
     31        (WebKit::WebPage::contextMenuHidden): Deleted.
     32        * WebProcess/WebPage/WebPage.cpp:
     33        (WebKit::WebPage::didShowContextMenu): Added.
     34        (WebKit::WebPage::didDismissContextMenu): Added.
     35        (WebKit::WebPage::mouseEvent):
     36        Drive-by: Rename `m_contextMenuShowing` to `m_waitingForContextMenuToShow` for clarity.
     37
    1382021-07-27  Myles C. Maxfield  <mmaxfield@apple.com>
    239

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

    r264333 r280374  
    7474
    7575    // No matter the result of showContextMenuWithItems, always notify the WebProcess that the menu is hidden so it starts handling mouse events again.
    76     page->send(Messages::WebPage::ContextMenuHidden());
     76    page->send(Messages::WebPage::DidShowContextMenu());
    7777}
    7878

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

    r280327 r280374  
    68296829    if (auto* automationSession = process().processPool().automationSession()) {
    68306830        if (m_controlledByAutomation && automationSession->isSimulatingUserInteraction()) {
    6831             send(Messages::WebPage::ContextMenuHidden());
     6831            send(Messages::WebPage::DidShowContextMenu());
    68326832            return;
    68336833        }
    … …  
    68436843
    68446844    m_activeContextMenu->show();
     6845}
     6846
     6847void WebPageProxy::didShowContextMenu()
     6848{
     6849    // Don't send `Messages::WebPage::DidShowContextMenu` as that should've already been eagerly
     6850    // sent when requesting the context menu to show, regardless of the result of that request.
     6851
     6852    pageClient().didShowContextMenu();
     6853}
     6854
     6855void WebPageProxy::didDismissContextMenu()
     6856{
     6857    send(Messages::WebPage::DidDismissContextMenu());
     6858
     6859    pageClient().didDismissContextMenu();
    68456860}
    68466861

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

    r280178 r280374  
    13151315#if ENABLE(CONTEXT_MENUS)
    13161316    // Called by the WebContextMenuProxy.
     1317    void didShowContextMenu();
     1318    void didDismissContextMenu();
    13171319    void contextMenuItemSelected(const WebContextMenuItemData&);
    13181320    void handleContextMenuKeyEvent();

  • trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm

    r279164 r280374  
    172172- (void)menuWillOpen:(NSMenu *)menu
    173173{
    174     _menuProxy->page()->pageClient().didShowContextMenu();
     174    _menuProxy->page()->didShowContextMenu();
    175175}
    176176
    177177- (void)menuDidClose:(NSMenu *)menu
    178178{
    179     _menuProxy->page()->pageClient().didDismissContextMenu();
     179    _menuProxy->page()->didDismissContextMenu();
    180180}
    181181

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

    r271206 r280374  
    6969
    7070    // Mark the WebPage has having a shown context menu then notify the UIProcess.
    71     m_page->contextMenuShowing();
     71    m_page->startWaitingForContextMenuToShow();
    7272    m_page->flushPendingEditorStateUpdate();
    7373    m_page->send(Messages::WebPageProxy::ShowContextMenu(contextMenuContextData, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));

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

    r280337 r280374  
    28582858};
    28592859
     2860#if ENABLE(CONTEXT_MENUS)
     2861
     2862void WebPage::didShowContextMenu()
     2863{
     2864    m_waitingForContextMenuToShow = false;
     2865}
     2866
     2867void WebPage::didDismissContextMenu()
     2868{
     2869    corePage()->contextMenuController().didDismissContextMenu();
     2870}
     2871
     2872#endif // ENABLE(CONTEXT_MENUS)
     2873
    28602874#if ENABLE(CONTEXT_MENU_EVENT)
    28612875static bool isContextClick(const PlatformMouseEvent& event)
    … …  
    29632977#if ENABLE(CONTEXT_MENUS)
    29642978    // Don't try to handle any pending mouse events if a context menu is showing.
    2965     if (m_isShowingContextMenu)
     2979    if (m_waitingForContextMenuToShow)
    29662980        shouldHandleEvent = false;
    29672981#endif

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

    r280337 r280374  
    10511051
    10521052#if ENABLE(CONTEXT_MENUS)
    1053     void contextMenuShowing() { m_isShowingContextMenu = true; }
     1053    void startWaitingForContextMenuToShow() { m_waitingForContextMenuToShow = true; }
    10541054#endif
    10551055
    … …  
    16211621
    16221622#if ENABLE(CONTEXT_MENUS)
    1623     void contextMenuHidden() { m_isShowingContextMenu = false; }
     1623    void didShowContextMenu();
     1624    void didDismissContextMenu();
    16241625#endif
    16251626#if ENABLE(CONTEXT_MENU_EVENT)
    … …  
    21732174
    21742175#if ENABLE(CONTEXT_MENUS)
    2175     bool m_isShowingContextMenu { false };
     2176    bool m_waitingForContextMenuToShow { false };
    21762177#endif
    21772178

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

    r280077 r280374  
    165165#endif
    166166
    167 #if ENABLE(CONTEXT_MENUS)
    168     ContextMenuHidden()
    169     ContextMenuForKeyEvent()
    170 #endif
    171 
    172167    ScrollBy(uint32_t scrollDirection, uint32_t scrollGranularity)
    173168    CenterSelectionInVisibleArea()
    … …  
    346341#if ENABLE(CONTEXT_MENUS)
    347342    # Context menu.
     343    DidShowContextMenu()
     344    DidDismissContextMenu()
    348345    DidSelectItemFromActiveContextMenu(WebKit::WebContextMenuItemData menuItem)
     346    ContextMenuForKeyEvent()
    349347#endif
    350348

  • trunk/Tools/ChangeLog

    r280366 r280374  
     12021-07-27  Devin Rousso  <drousso@apple.com>
     2
     3        [Modern Media Controls] [macOS] Overflow button still shows as `on` even after contextmenu is dismissed
     4        https://bugs.webkit.org/show_bug.cgi?id=228310
     5        <rdar://problem/81124786>
     6
     7        Reviewed by Wenson Hsieh.
     8
     9        * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
     10        (-[TestRunnerWKWebView dismissActiveMenu]):
     11        (-[TestRunnerWKWebView immediatelyDismissContextMenuIfNeeded]):
     12        (-[TestRunnerWKWebView _dismissAllContextMenuInteractions]): Added.
     13
    1142021-07-27  Diego Pino Garcia  <dpino@igalia.com>
    215

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

    r279750 r280374  
    186186{
    187187#if PLATFORM(IOS_FAMILY)
     188    [self _dismissAllContextMenuInteractions];
    188189    [self resignFirstResponder];
    189190#else
    … …  
    254255    self.showingContextMenu = NO;
    255256
     257    [self _dismissAllContextMenuInteractions];
     258}
     259
     260- (void)_dismissAllContextMenuInteractions
     261{
    256262#if PLATFORM(IOS)
    257263    for (id <UIInteraction> interaction in self.contentView.interactions) {
Note: See TracChangeset for help on using the changeset viewer.