Bug 228310 - [Modern Media Controls] [macOS] Overflow button still shows as `on` even after contextmenu is dismissed
Summary: [Modern Media Controls] [macOS] Overflow button still shows as `on` even afte...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-26 17:14 PDT by Devin Rousso
Modified: 2021-08-02 17:37 PDT (History)
11 users (show)

See Also:


Attachments
Patch (19.83 KB, patch)
2021-07-26 17:24 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (19.53 KB, patch)
2021-07-27 11:49 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (22.84 KB, patch)
2021-07-27 11:54 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (22.66 KB, patch)
2021-07-27 14:24 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2021-07-26 17:14:19 PDT
# STEPS TO REPRODUCE
1. go to any page with a `<video>`
2. click the >> overflow button
3. dismiss the contextmenu (e.g. click outside, escape, etc.)

# EXPECTED
the >> overflow button would no longer be white

# ACTUAL
the >> overflow button is still white
Comment 1 Devin Rousso 2021-07-26 17:16:28 PDT
<rdar://problem/81124786>
Comment 2 Devin Rousso 2021-07-26 17:24:31 PDT
Created attachment 434260 [details]
Patch
Comment 3 Wenson Hsieh 2021-07-26 17:37:54 PDT
Comment on attachment 434260 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434260&action=review

> Source/WebKit/WebProcess/WebPage/WebContextMenu.cpp:71
> +    m_page->shouldWaitForContextMenuToShow();

Nit - `shouldWaitForContextMenuToShow()` sounds a bit like the name of a const getter. Perhaps something along the lines of `startWaitingForContextMenuToShow()`?
Comment 4 Eric Carlson 2021-07-27 11:21:58 PDT
Comment on attachment 434260 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434260&action=review

> Source/WebCore/ChangeLog:21
> +        as that's the signal to the `MediaController` that the contextmenu interaction is ove, which

s/ove/over/
Comment 5 Devin Rousso 2021-07-27 11:49:46 PDT
Created attachment 434303 [details]
Patch
Comment 6 Devin Rousso 2021-07-27 11:54:07 PDT
Created attachment 434305 [details]
Patch

oops forgot new test files
Comment 7 Devin Rousso 2021-07-27 14:24:31 PDT
Created attachment 434315 [details]
Patch

fix test expectations
Comment 8 EWS 2021-07-27 20:56:52 PDT
Committed r280374 (240018@main): <https://commits.webkit.org/240018@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 434315 [details].