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

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].