[Modern Media Controls] add a way to show stats about the <video> #3463
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
EWS run on previous version of this PR (hash 3238e44) |
dcrousso
added
Media
|
|
webkit-ews-buildbot
added
the
merging-blocked
dcrousso
removed
the
merging-blocked
eric-carlson
approved these changes
Aug 18, 2022
| void HitTestResult::toggleShowMediaStats() const | ||
| { | ||
| #if ENABLE(VIDEO) | ||
| if (HTMLMediaElement* mediaElt = mediaElement()) |
There was a problem hiding this comment.
This isn't the first use of mediaElt in this file, but surely you can come up with a more descriptive variable name of the new code.
There was a problem hiding this comment.
lol yeah fair i can rename this
i didn't even really notice this cause it was mostly a copy pasta from "Loop" 😅
| bool HitTestResult::mediaStatsShowing() const | ||
| { | ||
| #if ENABLE(VIDEO) | ||
| if (HTMLMediaElement* mediaElt = mediaElement()) |
Comment on lines
+605
to
+606
| default: | ||
| break; |
There was a problem hiding this comment.
both cause i need to add a case for ContextMenuItemTagShowMediaStats and so that in the future if others add new items like that they don't even have to worry about handling it here
webkit-ews-buildbot
added
the
merging-blocked
dcrousso
removed
the
merging-blocked
webkit-early-warning-system
force-pushed
the
eng/244101
branch
2 times, most recently
from
2ca4b09
to
f57734c
Compare
https://bugs.webkit.org/show_bug.cgi?id=244101 <rdar://problem/98847210> Reviewed by Eric Carlson. This will allow developers to gleam more information about the current state of `<video>` at a glance, rather than having to use JS APIs to gather that info. This can also be expanded to shows information that doesn't have an equivalent JS API. * Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml: Add an experimental setting for this feature. * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::updatePreferences): If this new experimental setting is enabled, also require `TrackConfigurationEnabled` as that's needed for many of the stats to be shown. * Source/WebCore/Modules/modern-media-controls/controls/media-controls.css: (.stats-container): Added. (.stats-container > table): Added. (.stats-container > table > tr > :is(th, td)): Added. (.stats-container > table > tr > th): Added. * Source/WebCore/Modules/modern-media-controls/media/media-controller.js: (MediaController.prototype.setShowingStats): Added. (MediaController.prototype.setShowingStats.createRow): Added. When "Show Media Stats" is enabled, show a basic `<table>` containing rows for each stat that's centered inside the `<video>`. * Source/WebCore/Modules/modern-media-controls/media/overflow-support.js: (OverflowSupport.prototype.syncControl): * Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp: (WebCore::MediaControlsHost::showMediaControlsContextMenu): Also allow the (modern) media controls JS to provide `"includeShowMediaStats"`, which does the same acts the same as the general context menu item. * Source/WebCore/html/HTMLMediaElement.h: (WebCore::HTMLMediaElement::showingStats const): Added. The source of truth for "are we showing media stats" is in a member variable. * Source/WebCore/html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::setShowingStats): Added. Invoke some JS to create/remove the `<table>`, updating the member variable depending on the result. * Source/WebCore/platform/ContextMenuItem.h: * Source/WebCore/platform/ContextMenuItem.cpp: (WebCore::isValidContextMenuAction): * Source/WebCore/page/ContextMenuController.h: * Source/WebCore/page/ContextMenuController.cpp: (WebCore::ContextMenuController::showContextMenu): (WebCore::ContextMenuController::contextMenuItemSelected): (WebCore::ContextMenuController::addDebuggingItems): Renamed from `addInspectElementItem`. (WebCore::ContextMenuController::checkOrEnableIfNeeded const): * Source/WebCore/rendering/HitTestResult.h: * Source/WebCore/rendering/HitTestResult.cpp: (WebCore::HitTestResult::toggleShowMediaStats const): Added. (WebCore::HitTestResult::mediaStatsShowing const): Added. * Source/WebKit/UIProcess/API/Cocoa/WKMenuItemIdentifiersPrivate.h: * Source/WebKit/UIProcess/API/Cocoa/WKMenuItemIdentifiers.mm: * Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm: (WebKit::menuItemIdentifier): * Source/WebKitLegacy/mac/WebView/WebHTMLView.mm: (toTag): Create a new `ContextMenuItemTagShowMediaStats`, adding it after `ContextMenuItemTagInspectElement`. * Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm: (-[WKActionSheetAssistant _uiMenuElementsForMediaControlContextMenuItems:]): Add a way to create a separator in the (modern) media controls context menu on iOS. * Source/WebCore/en.lproj/modern-media-controls-localized-strings.js: * Source/WebCore/Modules/modern-media-controls/main.js: (UIString): Allow for multiple replacements in a single string. * Source/WebCore/en.lproj/Localizable.strings: * Source/WebCore/platform/LocalizedStrings.h: * Source/WebCore/platform/LocalizedStrings.cpp: (WebCore::contextMenuItemTagShowMediaStats): Added. Canonical link: https://commits.webkit.org/253583@main
webkit-early-warning-system
force-pushed
the
eng/244101
branch
from
f57734c
to
e5887c7
Compare
|
Committed 253583@main (e5887c7): https://commits.webkit.org/253583@main Reviewed commits have been landed. Closing PR #3463 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
e5887c7
25103a9
🛠 mac🛠 mac-debug🧪 ios-wk2🛠 mac-AS-debug🧪 api-ios🧪 api-mac🧪 mac-wk1🧪 mac-wk2🛠 🧪 merge🧪 mac-AS-debug-wk2🧪 mac-wk2-stress