Repository navigation
Media: Remove inline event handlers from the image editor - #14767
aeonframework wants to merge 1 commit into
Conversation
The image editor markup printed by `wp_image_editor()` bound every control
with an inline `on*` attribute (48 of them), and the "Edit Image" buttons
printed by `edit_form_image_editor()` and `get_media_item()` opened the editor
the same way. A Content Security Policy that allows scripts by nonce, without
`'unsafe-inline'`, blocks all of these, so the editor could not be opened and,
when printed with the page, its crop tool never initialized.
The controls now carry `data-imgedit-*` attributes and are bound by delegated
listeners in `image-edit.js`. The editor wrapper exposes the post ID, and the
nonce is read from the existing `imgedit-nonce-{id}` field instead of being
interpolated into JavaScript inside HTML attributes. The preview image's load
event is handled in the capture phase, with a check on DOM ready for an image
that loaded before the script ran.
See #59446.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @aeonframework. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
The image editor markup printed by
wp_image_editor()binds every control with an inlineon*attribute: 48 handlers covering crop, scale, rotate and flip, the popup menu keyboard navigation, undo and redo, save and cancel, the help toggles, the numeric crop and selection fields, and the preview image'sonload. The "Edit Image" buttons printed byedit_form_image_editor()andget_media_item()open the editor with an inlineonclicktoo.A site that sends a nonce-based Content Security Policy without
'unsafe-inline'blocks all of these (script-src-attr). Core already supports adding nonces to every script it prints, throughwp_script_attributesandwp_inline_script_attributes. Inline event handlers cannot carry a nonce, though. As a result:post.php?post=ID&action=edit), "Edit Image" does nothing.&image-editor, the editor is printed with the page, but the crop tool never initializes because the preview'sonloadis blocked.The image editor is the largest single group of inline handlers left in wp-admin. It had 48 of the 83
on*=attributes inwp-admin/andwp-includes/PHP at this base.Changes
wp-admin/includes/image-edit.php: every inline handler is replaced by adata-imgedit-actionattribute (buttons), adata-imgedit-inputattribute (numeric fields) or adata-imgedit-previewattribute (preview image). Adata-imgedit-valueattribute carries an argument where one is needed (rotation angle, flip axis,scale/restore, ratio axis)..imgedit-wrapgetsdata-postid. The nonce is read from the existing#imgedit-nonce-{id}field, so it is no longer interpolated into JavaScript inside HTML attributes.wp-admin/includes/media.php: both "Edit Image" buttons getdata-imgedit-open,data-postidanddata-nonce.js/_enqueues/lib/image-edit.js: delegated listeners ondocumentcall the sameimageEditmethods with the same arguments as before:[data-imgedit-open]and.imgedit-wrap [data-imgedit-action];keydown/bluron the rotate menu;keyup bluron[data-imgedit-input].imgLoaded()runs exactly once either way. The images thatrefreshEditor()builds are unaffected.imageEditmethod signatures, element IDs and markup structure are unchanged. Code that triggers clicks on these elements keeps working because the listeners are delegated.Testing
New
tests/phpunit/tests/admin/includesImageEdit.php, 3 tests:wp_image_editor()andedit_form_image_editor()output has noon*attribute (checked withWP_HTML_Tag_Processor);data-postidmatches the attachment, and the nonce fields verify forimage_editor-{id}.Before the change all 3 tests fail. After it,
vendor/bin/phpunit tests/phpunit/tests/admin/includesImageEdit.phpreports OK (3 tests, 26 assertions).Full PHPUnit suite,
vendor/bin/phpunitagainst MySQL 8.0 on PHP 8.3:Tests_Template(23),Tests_Dependencies_jQuery(4) andTests_Script_Modules_WpScriptModules(1), and they come from the environment (thesrc/tree was not built).vendor/bin/phpcson the changed PHP files: no new warnings. The 3 existing warnings inmedia.phpare unchanged.JSHint 2.13.6 with the repo's
.jshintrconimage-edit.js: nothing new. The only 2 warnings are the existing==at line 1035.Browser check: headless Chromium against a 7.2-beta1 nightly with the three changed files copied in,
SCRIPT_DEBUGon,CONCATENATE_SCRIPTSoff. A mu-plugin sendsContent-Security-Policy: script-src 'self' 'nonce-…'and adds the nonce throughwp_script_attributesandwp_inline_script_attributes.script-src-attr) and the editor never opens. With&image-editorthe crop tool never initializes. Threescript-src-attrviolations came from the editor.&image-editor: crop initialized, Cancel closes.<script>tags inwp-admin/includes/misc.php(_wpColorScheme,wp_admin_canonical_url(), the back-forward cache reload) and the TinyMCE init inclass-wp-editor.php.Follow-ups (not in this PR)
misc.php's three admin-head scripts go throughwp_print_inline_script_tag(), the same pattern as r63545.media-upload.phpscreens (get_media_item()Delete/Cancel andWPSetAsThumbnail, and theaddExtImageform), inmeta-boxes.phpand in the list tables.Trac ticket: https://core.trac.wordpress.org/ticket/59446
Use of AI Tools
AI assistance: Yes
Tool(s): MiniAeon (omp agent harness)
Model(s): Claude Opus 5.5
Used for: finding the remaining inline handlers, drafting the markup and listener changes, the PHPUnit tests and the headless-browser CSP check. The tests and the headless-browser CSP check listed above were run during that work, and the full diff was reviewed by a person before this PR was opened.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.