Bug 230761 - [Media Controls] Allow for a close button
Summary: [Media Controls] Allow for a close button
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-24 10:59 PDT by Antoine Quint
Modified: 2021-09-24 15:36 PDT (History)
12 users (show)

See Also:

Attachments
Patch (32.91 KB, patch)
2021-09-24 11:00 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Patch (16.68 KB, patch)
2021-09-24 11:19 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Patch (17.53 KB, patch)
2021-09-24 14:24 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2021-09-24 10:59:10 PDT 
[Media Controls] Allow for a close button
Comment 1 Antoine Quint 2021-09-24 11:00:53 PDT 
Created attachment 439168 [details]
Patch
Comment 2 Antoine Quint 2021-09-24 11:19:48 PDT 
Created attachment 439174 [details]
Patch
Comment 3 Antoine Quint 2021-09-24 11:44:20 PDT 
Committed r283050 (242110@main): <https://commits.webkit.org/242110@main>
Comment 4 Radar WebKit Bug Importer 2021-09-24 11:45:44 PDT 
<rdar://problem/83505067>
Comment 5 Eric Hutchison 2021-09-24 13:29:53 PDT 
Reverted r283050 for reason:

Broke commit-queue

Committed r283054 (242114@main): <https://commits.webkit.org/242114@main>
Comment 6 Antoine Quint 2021-09-24 14:24:38 PDT 
Created attachment 439197 [details]
Patch
Comment 7 Devin Rousso 2021-09-24 14:48:01 PDT 
Comment on attachment 439197 [details]
Patch

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

> Source/WebCore/Modules/modern-media-controls/media/close-support.js:38
> +        this.mediaController.media.webkitExitFullscreen();

Can we not use `FullscreenButton` and `FullscreenSupport` to achieve this same behavior?  Why do we need an entirely new button that is just a functional subset of an existing button with a different icon?  Could we parameterize the existing button instead to just change the icon based on a `LayoutTraits` getter?
Comment 8 EWS 2021-09-24 15:35:57 PDT 
Committed r283064 (242122@main): <https://commits.webkit.org/242122@main>

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