Skip to content

Media: Stop the shell breaking core's uploader button and grid refresh - #487

Merged
mmtr merged 4 commits into
trunkfrom
claude/media-screen-dialog-bug-77c3ca
Aug 4, 2026
Merged

Media: Stop the shell breaking core's uploader button and grid refresh#487
mmtr merged 4 commits into
trunkfrom
claude/media-screen-dialog-bug-77c3ca

Conversation

@mmtr

@mmtr mmtr commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Two Media Library bugs, both from shell plumbing leaking into a core screen.

Proposed Changes

  • The chromeless bridge's link interceptor now skips anchors carrying core's aria-button-if-js class. Fixes Add Media File opening a redundant window on top of core's inline uploader, and restores the comment row actions and the plugin auto-update toggle, which broke the same way.
  • Links with that class but no owning script (the Media list table's Trash / Restore / Delete Permanently) get _wp_http_referer stamped on the href before the interceptor yields.
  • openstation_chromeless is stripped from the query vars upload.php hands the media grid. Fixes the grid not showing an upload until the window is reopened.
  • New vitest harness that runs the emitted interceptor against a fixture DOM.

Incidentally fixes cancelling the "are you sure" confirm on Delete Permanently. The interceptor used to preventDefault in capture and hand the URL to the shell before the inline onclick ran, so cancelling still deleted the file.

Why are these changes being made?

aria-button-if-js is core's marker for "this anchor is an in-page button, the href is only the no-JS fallback". Our interceptor is capture-phase, so it beat the script that owns the button and gave the user the fallback URL instead of the action they clicked. Skipping on the class covers every surface with that shape rather than just this one button.

The class doesn't promise a handler, though. The Media list table stamps it and binds nothing, so those hrefs really are the navigation, and the parent used to add _wp_http_referer to them on its destructive-action path. Yielding loses that, and under a Referrer-Policy of strict-origin or tighter post.php then sees a bare origin, matches neither post.php nor post-new.php, and redirects the window to the site front page. Stamping it in the iframe is simpler than the parent round trip: the source page is just window.location.

The interceptor is JS emitted from a PHP heredoc, so the PHP suite can only assert that source strings appear in the right order. Both bugs of this shape got through that, hence the DOM-level harness.

upload.php copies unknown $_GET keys into _wpMediaGridSettings.queryVars, and wp.media.model.Query only watches the upload queue when every query arg is one it can filter on. Our flag riding along was enough to stop the grid from ever seeing a finished upload. Core has no filter on that array, so the shim re-localizes the settings with the key removed.

Testing Instructions

Media grid, both fixes:

  1. Enable OpenStation and open the Media window in grid view.
  2. Click Add Media File. Make sure only the inline uploader expands. No second window opens.
  3. Click Select Files and pick an image. Make sure the new item appears in the grid as soon as the upload finishes, without reopening the window.
  4. Click Add Media File again. Make sure the uploader collapses.

On trunk, step 2 also opens an Add Media File window (closing it leaves the drop zone stranded above the grid), and step 3 uploads the file but never shows it.

Media list table, where core stamps the class but binds no handler:

  1. Switch the Media window to list view (upload.php?mode=list).
  2. Hover a row, click Delete Permanently (or Trash if MEDIA_TRASH is on), then cancel the confirm. Make sure the file is still there.
  3. Click it again and confirm this time.
  4. Make sure the window lands back on the media list with the "Media file permanently deleted" notice, still chromeless, in the same window. No new window, no admin sidebar in the iframe.
  5. Worth repeating step 3 with a stricter referrer policy, since the default one hides the failure. Drop a mu-plugin that sends Referrer-Policy: strict-origin on admin responses, then check the window still lands on the media list rather than the site front page.

Comment row actions:

  1. Leave a comment on any post, open the Comments window.
  2. Hover the row and click Unapprove. Make sure the row turns yellow and Pending goes up in place, with no reload and no new window.

Plugin auto-update toggle:

  1. Open the Plugins window and click Enable auto-updates on any plugin.
  2. Make sure the link flips to Disable auto-updates in place, with no reload and no new window.

Regression check on the existing skip list:

  1. Open the Plugins window, go to Add New Plugin, click Install Now on any plugin.
  2. Make sure it installs in place with the inline progress label, not by opening a window on update.php.
Open WordPress Playground Preview

@mmtr mmtr self-assigned this Aug 4, 2026
The bridge's link interceptor runs in the capture phase, so it
preventDefaults before the script that owns an in-page button gets the
click. On the Media Library grid that opened a window for media-new.php
(the anchor's no-JS fallback) while media-grid.js still expanded the
inline uploader in the Media window behind it.

Bail on anchors carrying core's aria-button-if-js class, which is core's
own marker for "this href is only the no-JS fallback". Also restores the
comment row actions and the plugin auto-update toggle, which the
interceptor was hijacking the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mmtr
mmtr force-pushed the claude/media-screen-dialog-bug-77c3ca branch from 82b040e to 809a559 Compare August 4, 2026 15:11
upload.php copies unknown $_GET keys into _wpMediaGridSettings.queryVars,
so openstation_chromeless=1 ended up as a media query arg. Core's
wp.media.model.Query only observes wp.Uploader.queue when every arg is
one it can filter on, so the observer was never attached and a finished
upload never entered the grid's collection: the file uploaded, the grid
showed nothing, and the item only appeared after reopening the window.

Re-localize _wpMediaGridSettings after upload.php with our key removed.
Read back what core produced rather than recomputing it, so the shim
doesn't drift if core changes the shape.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mmtr mmtr changed the title Chromeless bridge: Stop hijacking core's JS-button links Media: Stop the shell breaking core's uploader button and grid refresh Aug 4, 2026
mmtr and others added 2 commits August 4, 2026 18:34
The aria-button-if-js bail swallowed the Media list table's Trash /
Restore / Delete Permanently links, which core stamps with the class
but binds no handler to. They used to reach the parent's
destructive-action path and get _wp_http_referer added; yielding meant
a raw navigation without it. Under a Referrer-Policy of strict-origin
or tighter, post.php then sees a bare origin, matches neither post.php
nor post-new.php, and redirects the window to the site front page.

Stamp the hint in the iframe instead, where the source page is just
window.location. Yielding stays correct for these links: the inline
onclick confirm now runs before anything happens, so cancelling
actually cancels, which it did not when we preventDefaulted ahead of
it in capture.

Adds a vitest harness that runs the emitted interceptor against a
fixture DOM, since the PHP suite can only assert on source text and
both regressions of this shape got through that. Also corrects the
docblock reference to tags.js (there is no edit-tags.js), decodes the
media-grid settings as objects so nested values can't collapse to [],
and rewrites the screen-guard test, which passed with the guard
removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mmtr
mmtr merged commit 0d0e87f into trunk Aug 4, 2026
5 checks passed
@mmtr
mmtr deleted the claude/media-screen-dialog-bug-77c3ca branch August 4, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant