Repository navigation
fix: stem mixer on iOS — install play/pause shims via defineProperty - #17
Merged
Merged
Conversation
iOS WebKit exposes HTMLMediaElement.play/.pause as non-writable, so the `core.play = fn` / `core.pause = fn` assignments in installAudioShims() threw "Attempted to assign to readonly property" in the plugin's strict-mode module context. That left playOk/pauseOk false → shimsUsable false, so onSongReady() refused the sloppak takeover and the browser played only stems[0] (single stem, dead mixer sliders). Chromium/Electron allowed the assignment, so only iOS was affected. Install play/pause with Object.defineProperty (own property on the instance), which works even when the inherited method is non-writable — matching the currentTime/paused/duration shims. Adds a regression test and bumps to 0.7.1. Closes #16 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
There was a problem hiding this comment.
Pull request overview
This PR fixes iOS WKWebView stem-mixer takeover failures by installing #audio play/pause shims via Object.defineProperty, avoiding strict-mode assignment errors on non-writable WebKit media methods.
Changes:
- Update
installAudioShims()to definecore.play/core.pauseviaObject.defineProperty(consistent with other shims). - Add a regression/unit test covering the iOS non-writable method override behavior and guarding against reintroducing the fragile assignment pattern.
- Bump plugin version to
0.7.1and document the fix in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/ios-play-pause-shim.test.mjs |
Adds regression coverage ensuring play/pause shims use defineProperty and validates behavior on a non-writable-method model. |
screen.js |
Switches play/pause shim installation from direct assignment to Object.defineProperty to work on iOS WebKit. |
plugin.json |
Bumps plugin version to 0.7.1. |
CHANGELOG.md |
Documents the iOS stem playback fix for 0.7.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot review on PR #17 flagged the assignment-pattern guard as too narrow: it only matched `core.play = function` / `core.pause = function` (literal "function" keyword, single-quoted defineProperty target), so a regression using an arrow function or a differently-quoted defineProperty call would slip through undetected while still throwing on iOS WebKit. Broaden the guard to reject any `core.play =` / `core.pause =` assignment (after stripping full-line comments, since the neighboring comment in screen.js literally contains the string "core.play = fn" to explain the fix), and accept either quote style in the defineProperty assertions. Verified: manually mutated screen.js to reintroduce the bug via an arrow function assignment and confirmed the tightened guard now fails as expected; restored and re-ran full suite (9/9 green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
This file contains hidden or 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
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.
Closes #16.
Problem
On iOS (iPhone/iPad, run through the native
feedback-client-appWKWebView) a multi-stem sloppak plays only one stem and the mixer sliders do nothing. Desktop/Electron unaffected.installAudioShims()installed the#audioplay/pauseshims by direct assignment (core.play = fn). iOS WebKit makes those methods non-writable, so the assignment throws "Attempted to assign to readonly property" in the plugin's strict-mode ES-module context.playOk/pauseOkstay false →shimsUsablefalse →onSongReady()aborts the takeover and hands playback to the native<audio>, which only playsstems[0]. Proxy logs from the iOS client confirmed onlystems/guitar.ogg(=stems[0]) was ever fetched.Fix
Install
play/pausewithObject.defineProperty(core, …, { configurable: true, writable: true, value: fn })— an own property on the instance, which succeeds even when the inherited method is non-writable, on both WebKit and Chromium. This matches the existingcurrentTime/paused/durationshims (alreadydefineProperty, already working).Tests
tests/ios-play-pause-shim.test.mjs:screen.jsno longer uses the fragilecore.play = function/core.pause = functionassignment and usesdefinePropertyfor both;definePropertyoverrides a non-writable method on an iOS-style element where a plain assignment throws.node --test→ all green (9/9).Notes
plugin.jsonbumped to 0.7.1.