feat(mac/v3): expose mediaTypesRequiringUserActionForPlayback on WKWebView#5513
feat(mac/v3): expose mediaTypesRequiringUserActionForPlayback on WKWebView#5513Eyalm321 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR adds media autoplay support to macOS WebViews by introducing a new ChangesmacOS WebView media autoplay support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0ba4388 to
c0d1c9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/build-macos-media-playback.yml (3)
12-12: 💤 Low valueJob conditions are redundant with branch triggers.
The
if: contains(github.ref, 'v2')andif: contains(github.ref, 'v3')conditions are redundant given the workflow only triggers on the two specific branches. If you keep both the branch trigger and job conditions, consider using exact matching likegithub.ref == 'refs/heads/fix/macos-media-playback-options-v2'instead of substring matching to avoid potential false positives if branch naming changes.Also applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-macos-media-playback.yml at line 12, The job-level conditional "if: contains(github.ref, 'v2')" (and the similar "if: contains(github.ref, 'v3')") is redundant with the workflow's branch triggers; remove these contains(...) job conditions from the workflow so the branch filters defined in the trigger handle selection, or if you must keep a job guard replace the substring match with an exact ref equality check (e.g., use "github.ref == 'refs/heads/your-branch-name'" for the specific branch) to avoid false positives—update the two occurrences of the job-level if conditions accordingly.
1-54: ⚡ Quick winConsider security hardening for the workflow.
Static analysis identified several security posture gaps that, while not immediately exploitable, are recommended best practices for GitHub Actions workflows:
Credential persistence (lines 17, 42): The
actions/checkoutstep does not setpersist-credentials: false, which means the GitHub token persists in the workspace and could be accessed by subsequent steps.Overly broad permissions (workflow-level): No explicit
permissions:block is defined, so the workflow inherits default repository permissions. Best practice is to grant only the minimum required permissions (likely justcontents: readfor this build workflow).Unpinned action references (lines 17, 19, 42, 44): Actions reference tags (
@v4) rather than commit SHAs. Pinning to commit hashes prevents supply chain attacks if action repositories are compromised.For a temporary feature-branch build workflow, these improvements are optional but recommended if this workflow will be promoted to a long-lived or production CI pipeline.
🔒 Security hardening example
name: Build darwin (media-playback options) +permissions: + contents: read + on: push:- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: falseFor action pinning, you would replace version tags with commit SHAs (example shown for illustration; verify current commits):
- - uses: actions/checkout@v4 + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-macos-media-playback.yml around lines 1 - 54, Add minimal workflow security hardening: set persist-credentials: false on both steps that call actions/checkout@v4 (the checkout steps), add a top-level permissions block granting only contents: read, and replace the unpinned action refs (actions/checkout@v4 and actions/setup-go@v4) with pinned commit SHAs (or another strong pinning mechanism) to avoid supply-chain risks; update the checkout and setup-go step entries accordingly so the workflow uses persist-credentials: false, the minimal permissions block, and SHA-pinned action refs.
3-8: Consider the lifecycle of this workflow after the feature branches merge.The workflow triggers on specific feature branch names (
fix/macos-media-playback-options-v2andfix/macos-media-playback-options-v3). Once these branches are merged and deleted, this workflow file will remain in the repository but never trigger. Consider either:
- Using a branch pattern (e.g.,
fix/macos-media-playback-*) if similar development branches are expected- Removing this workflow file after the feature is merged, or
- Converting this to a reusable workflow that can be triggered manually for testing macOS builds
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-macos-media-playback.yml around lines 3 - 8, The workflow is pinned to two specific branch names (fix/macos-media-playback-options-v2 and fix/macos-media-playback-options-v3) which will stop triggering after those branches are merged; update the workflow trigger in the on: push block to use a branch pattern like fix/macos-media-playback-* or convert the file into a reusable/workflow_call workflow or delete the workflow file after the feature is merged so it does not become a dead workflow entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-macos-media-playback.yml:
- Line 19: Replace the outdated GitHub Action reference "uses:
actions/setup-go@v4" with the current major "uses: actions/setup-go@v6"
throughout the workflow so the job that configures Go uses the supported
release; search for occurrences of the literal string actions/setup-go@v4 and
update them to actions/setup-go@v6 in the workflow step(s) that set up Go (the
step currently showing uses: actions/setup-go@v4).
---
Nitpick comments:
In @.github/workflows/build-macos-media-playback.yml:
- Line 12: The job-level conditional "if: contains(github.ref, 'v2')" (and the
similar "if: contains(github.ref, 'v3')") is redundant with the workflow's
branch triggers; remove these contains(...) job conditions from the workflow so
the branch filters defined in the trigger handle selection, or if you must keep
a job guard replace the substring match with an exact ref equality check (e.g.,
use "github.ref == 'refs/heads/your-branch-name'" for the specific branch) to
avoid false positives—update the two occurrences of the job-level if conditions
accordingly.
- Around line 1-54: Add minimal workflow security hardening: set
persist-credentials: false on both steps that call actions/checkout@v4 (the
checkout steps), add a top-level permissions block granting only contents: read,
and replace the unpinned action refs (actions/checkout@v4 and
actions/setup-go@v4) with pinned commit SHAs (or another strong pinning
mechanism) to avoid supply-chain risks; update the checkout and setup-go step
entries accordingly so the workflow uses persist-credentials: false, the minimal
permissions block, and SHA-pinned action refs.
- Around line 3-8: The workflow is pinned to two specific branch names
(fix/macos-media-playback-options-v2 and fix/macos-media-playback-options-v3)
which will stop triggering after those branches are merged; update the workflow
trigger in the on: push block to use a branch pattern like
fix/macos-media-playback-* or convert the file into a reusable/workflow_call
workflow or delete the workflow file after the feature is merged so it does not
become a dead workflow entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 77eeaeec-a650-4164-a5d1-1778a34edea1
📒 Files selected for processing (3)
.github/workflows/build-macos-media-playback.ymlv3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_options.go
Add EnableAutoplayWithoutUserAction to MacWebviewPreferences, mapping to WKWebViewConfiguration.mediaTypesRequiringUserActionForPlayback. Without this option, there is no way to autoplay HTML5 video/audio in a Wails v3 macOS app — every <video autoplay> is blocked by WebKit's user-gesture requirement. Follows the existing pattern used for TabFocusesLinks, AllowsBackForwardNavigationGestures etc.: u.Bool option, bool* in the C struct, applied on WKWebViewConfiguration inside windowNew when set. Defaults preserve current WebKit behaviour (gesture required) — opt-in only. iOS already does the equivalent in webview_window_ios.m driven by EnableAutoplayWithoutUserAction in application_options.go; this brings v3 macOS to parity. Closes wailsapp#5511
c0d1c9e to
faf027f
Compare
Summary
Adds
EnableAutoplayWithoutUserActiontoMacWebviewPreferences, mapping toWKWebViewConfiguration.mediaTypesRequiringUserActionForPlayback. Without it, there is no way to autoplay HTML5 video/audio in a Wails v3 macOS app — every<video autoplay>is blocked.Closes #5511. Companion v2 PR: #5512.
Implementation
Mirrors the existing preference-passing pattern (compare
TabFocusesLinks,AllowsBackForwardNavigationGestures):EnableAutoplayWithoutUserAction u.BooltoMacWebviewPreferences(webview_window_options.go)struct WebviewPreferenceswith the matching pointer field (webview_window_darwin.go)getWebviewPreferences()viaIsSet()+bool2CboolPtr(...)WKWebViewConfigurationinsidewindowNew():Defaults preserve WebKit's current behaviour (user gesture required) — opt-in only.
iOS already does the equivalent in
webview_window_ios.mdriven byEnableAutoplayWithoutUserActioninapplication_options.go; this brings macOS to parity.Note:
allowsInlineMediaPlaybackis iOS-only onWKWebViewConfiguration(verified by macos-latest compile failure), so it is intentionally not exposed on macOS — videos always play inline on macOS WKWebView anyway.Test plan
macos-latest/ Xcode 16.4)<video autoplay src="...">andMacWebviewPreferences{EnableAutoplayWithoutUserAction: u.True}autoplays without a clickSummary by CodeRabbit