Make WordPress Core

Opened 5 weeks ago

Last modified 5 weeks ago

#65816 new defect (bug)

Gallery block: Uncaught TypeError when trying to edit a gallery image in the Image editor

Reported by: afercia Owned by:
Priority: normal Milestone: Awaiting Review
Component: Editor Version:
Severity: normal Keywords: has-screenshots
Cc: Focuses:

Description

Steps to reproduce:

  • Add a gallery block with a few images.
  • Save.
  • Select the gallery block (not a single image).
  • In the gallery block toolbar, click 'Add' then click 'Open Media Library'.
  • The media modal dialog opens, the displayed media frame is 'Add to gallery'.
  • In the left panel, click 'Edit gallery'.
  • The displayed media frame is now the 'Edit gallery' one.
  • Click one of the images.
  • Observe the right panel 'Attachment details' appears.
  • Click 'Edit Image' in the 'Attachment details' panel.
  • Expected: the Image editor to open.
  • Actual: the media modal switches to an inconsistent frame that shows a title 'Edit image' but still shows all the images in the gallery. No image editor appears.
  • Observe the error message in your browser console:

Uncaught TypeError: Cannot read properties of undefined (reading 'toJSON')

I was able to reproduce this issue back till WordPress 5.6. Although the flow to get to the Edit gallery frame is a little different, the error can be reproduced in all versions till 5.6. Maybe even before 5.6, but I stopped my time travel there.

See attached screenshot.

Attachments (1)

01 typeerror.png (1.1 MB ) - added by afercia 5 weeks ago.

Download all attachments as: .zip

Change History (2)

@afercia
5 weeks ago

#1 @bejignesh
5 weeks ago

Reproduced on trunk, 7.1-RC1-63034-src, following the steps exactly.

The throw comes from @wordpress/media-utils, not from core's media views:

EditImage.prepare      media-views.js:5422   return this.model.toJSON();
child.editState        js/dist/media-utils.js:1038

getGalleryDetailsMediaFrame() binds its own handler for content:render:edit-image that reads the selection from the gallery state:

editState() {
    const selection = this.state( 'gallery' ).get( 'selection' );
    const view = new wp.media.view.EditImage( {
        model: selection.single(),
        controller: this,
    } ).render();
    ...
}

gallery is the "Create gallery" library, and it only holds a selection while a gallery is being built from scratch. Entering through "Edit gallery" never uses it, so single() is undefined and EditImage.prepare() throws. State at the moment of the crash:

state selection length single() image
gallery 0 undefined
gallery-edit 1 15
edit-image 15

The image that was clicked is available in two places. The handler reads the one place it is not.

Three handlers are bound to that event, in this order: editState from media-utils, then core's editImageContent twice. Because the first throws, Backbone's dispatch aborts and core's handlers never run, which is why the frame is left showing the gallery under an "Edit image" title.

Core's editImageContent reads this.state().get( 'image' ), which is what wp.media.view.Attachment.Details.editAttachment() sets just before switching state, and it is correct on both entry paths. Removing the media-utils override makes the reported flow work, and the "Create gallery" flow that already worked is unaffected. I checked both.

src/wp-includes/js/dist/media-utils.js is a prebuilt Gutenberg artifact, pinned in package.json at f05e40e91c54f29c449b1f33d0db89f5166812d9, so this is not fixable in core. Patch sent upstream: https://github.com/WordPress/gutenberg/pull/81285

Separate from this ticket, and noticed while tracing it: MediaFrame.Select.bindHandlers() binds content:render:edit-image to editImageContent, and MediaFrame.Post.bindHandlers() binds the same pair again through its handlers map, so the handler is registered twice on the post frame. In the Classic block's Add Media modal, which involves no Gutenberg media frame at all, one click on Edit Image fires two action=image-editor&do=open requests. I am opening a separate ticket for that rather than mixing it in here.

Note: See TracTickets for help on using tickets.