Skip to content

Playlist: Disable waveform player auto-init - #81489

Closed
scruffian wants to merge 4 commits into
trunkfrom
scruffian/fix/waveform-autoinit
Closed

scruffian wants to merge 4 commits into
trunkfrom
scruffian/fix/waveform-autoinit

Conversation

@scruffian

@scruffian scruffian commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

What?

Disables waveform-player auto-initialization and initializes playlist waveforms explicitly from the Playlist block code path.

Why?

The waveform-player dependency scans the document for [data-waveform-player] elements when it loads. That means any matching markup already on the page can be initialized outside the Playlist block's control. The Playlist block should own initialization so it can avoid unexpected behavior from arbitrary markup.

How?

  • Updates @arraypress/waveform-player to a version that supports disabling auto-init.
  • Adds data-waveform-autoinit="false" to the document element before importing the waveform dependency.
  • Keeps Playlist block initialization explicit through the existing initWaveformPlayer utility.
  • Sanitizes custom SVG icons before passing them to waveform-player.

Testing Instructions

Automated tests

Run:

npm run test:unit -- packages/block-library/src/utils/test/waveform-player-dependency.js packages/block-library/src/utils/test/waveform-utils.js
npm run lint:js -- packages/block-library/src/utils/waveform-utils.js packages/block-library/src/utils/disable-waveform-autoinit.js packages/block-library/src/utils/test/waveform-player-dependency.js packages/block-library/src/utils/test/waveform-utils.js

Manual testing

  1. Start the development environment:
npm run env start
  1. Open the post editor and insert a normal Playlist block.
  2. Add an audio track to the Playlist block so the Playlist frontend script loads.
  3. Add a separate Custom HTML block to the same post with this fixture:
<div
	id="waveform-autoinit-check"
	data-waveform-player
	data-url="https://example.invalid/unsafe-fixture.mp3"
	data-title="Unsafe fixture"
	data-button-align="unsafe-console-signal"
	data-show-playback-speed="true"
	data-playback-rates='["unsafe-console-signal"]'
	data-play-icon="&lt;img src=x-invalid-waveform-icon onerror=&quot;console.log( 'Unsafe waveform icon onerror ran' ); window.__unsafeWaveformIconRan = true&quot;&gt;"
	data-pause-icon="&lt;img src=x-invalid-waveform-icon onerror=&quot;console.log( 'Unsafe waveform icon onerror ran' ); window.__unsafeWaveformIconRan = true&quot;&gt;"
></div>
  1. Save and preview the post with the browser console open. Reload the preview if needed so console output from page load is visible.
  2. Confirm the real Playlist block still renders a waveform player and can play, pause, and switch tracks.
  3. Confirm the Custom HTML fixture was not auto-initialized:
const fixture = document.querySelector( '#waveform-autoinit-check' );
typeof window.WaveformPlayer; // Expected "function"
fixture.dataset.waveformInitialized; // Expected undefined
fixture.querySelector( '.waveform-player-inner' ); // Expected null
window.__unsafeWaveformIconRan; // Expected undefined

Also confirm the console has no messages beginning with [WaveformPlayer] Invalid or Unsafe waveform icon.

On trunk with only @arraypress/waveform-player updated to 1.26.0, this manual test should fail because the Custom HTML fixture is auto-initialized. The console should show output during page load, including:

[WaveformPlayer] Invalid buttonAlign option, using default: unsafe-console-signal
[WaveformPlayer] Invalid playbackRates attribute: ["unsafe-console-signal"]
Unsafe waveform icon onerror ran

The console check should show:

const fixture = document.querySelector( '#waveform-autoinit-check' );
typeof window.WaveformPlayer; // Expected "function"
fixture.dataset.waveformInitialized; // Expected "true" on the vulnerable build
fixture.querySelector( '.waveform-player-inner' ); // Expected an element on the vulnerable build
window.__unsafeWaveformIconRan; // Expected true on the vulnerable build

Screenshots or screencast

N/A

@github-actions github-actions Bot added the [Package] Block library /packages/block-library label Aug 12, 2026
@github-actions

Copy link
Copy Markdown

Size Change: +3.72 kB (+0.05%)

Total Size: 7.72 MB

📦 View Changed
Filename Size Change
build/modules/block-library/playlist/view.min.js 18.9 kB +1.77 kB (+10.32%) ⚠️
build/scripts/block-library/index.min.js 359 kB +1.95 kB (+0.55%)

compressed-size-action

@scruffian
scruffian marked this pull request as ready for review August 12, 2026 10:18
@github-actions

github-actions Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: scruffian <scruffian@git.wordpress.org>
Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@scruffian scruffian added [Type] Enhancement A suggestion for improvement. [Block] Playlist labels Aug 12, 2026
@scruffian scruffian self-assigned this Aug 12, 2026
@scruffian scruffian added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Aug 12, 2026
'1.5x',
'1.75x',
'2x',
];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this because waveform player now validates this array.

@scruffian
scruffian force-pushed the scruffian/fix/waveform-autoinit branch from df2e2ab to a60995d Compare August 12, 2026 10:34
@scruffian
scruffian requested a review from jeryj August 12, 2026 11:47
@github-actions

Copy link
Copy Markdown

Flaky tests detected in 372ae29.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/31591337669
📝 Reported tests:

All navigation links should default to the body color and submenus and mobile overlay should default to a white background with black text in /test/e2e/specs/editor/blocks/navigation-colors.spec.js, passed after 1 failed attempt.
Error: apiRequestContext.fetch: socket hang up
Call log:
  - → GET http://localhost:8889/wp-json/wp/v2/navigation/
    - user-agent: Playwright/1.62.1 (x64; ubuntu 24.04) node/20.20 CI/1
    - accept: */*
    - accept-encoding: gzip,deflate,br
    - X-WP-Nonce: c51ef1bc21
    - content-type: application/json
    - content-length: 90
    - cookie: wordpress_test_cookie=WP%20Cookie%20check; wordpress_logged_in_23778236db82f19306f247e20a353a99=admin%7C1786706761%7CxGzCHzEm3Kj5aCqNXTuqdBHY3QbY22omwUa3mk0YjHN%7C09c7ab43f622b2cc80b717b7182180bb0631bdacc19998684f405d9d13b5c890; wp-settings-time-1=1786534136

    at RequestUtils.rest (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/rest.ts:112:39)
    at RequestUtils.deleteAllMenus (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/menus.ts:74:30)
    at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-colors.spec.js:37:17

Comment thread patches/README.md

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this PR we will have no patches in the repo, let us remove the patches directory after rebase.

@manzoorwanijk manzoorwanijk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code has some feedback.

const element = createDeclarativePlayer();
it( 'initializes declarative players only after an explicit request from the Playlist utility', () => {
const element = createDeclarativePlayer( {
'data-play-icon':

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This payload is never asserted. In 1.26.0 parseDataAttributes reads data-play-icon (src/js/utils.js:397) and core.js:330 interpolates it unescaped into innerHTML, so after init() the <img onerror> is actually rendered. Either assert it isn't (it would fail today) or drop the payload so the test doesn't imply coverage it lacks.

if ( waveformGradient ) {
container.setAttribute( 'data-waveform-gradient', waveformGradient );
}
const sanitizedPlayIcon = playIcon && sanitizeSvgIcon( playIcon );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing passes playIcon / pauseIcon today (playlist/view.js and utils/waveform-player.js don't), so sanitizeSvgIcon is unused in production. Maybe split it into its own PR once there's a consumer?

Nit: startsWith( 'on' ) at L274 is redundant, the ICON_ATTRIBUTES allowlist already excludes every on* name.

### Bug Fixes

- Footnotes: Treat unreadable `footnotes` post meta as no footnotes instead of throwing, so the block shows its placeholder rather than the block crash warning ([#81201](https://github.com/WordPress/gutenberg/pull/81201)).
- Playlist: Disable waveform player auto-initialization before loading the dependency, and sanitize custom SVG icons passed through the Playlist waveform utility.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the PR reference ([#81489](https://github.com/WordPress/gutenberg/pull/81489)). Also, the sanitization half describes code nothing calls yet.

@manzoorwanijk

Copy link
Copy Markdown
Member

A gentle request, can we please prioritize this to get rid of the patch?

@ciampo

ciampo commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

I believe we can close this PR in favour of #83132.

Feel free to re-open in case I misunderstood 🙏

@ciampo ciampo closed this Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Playlist [Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants