Make WordPress Core

Opened 5 years ago

Last modified 2 years ago

#49431 new defect (bug)

Media uploader window leaves behind orphan button element in Block Editor

Reported by: talldanwp's profile talldanwp Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.0
Component: Media Keywords:
Focuses: javascript Cc:

Description

This issue was originally reported in the Gutenberg Github repo - https://github.com/WordPress/gutenberg/issues/9184. Reporting this here as well—currently it looks like a fix might need to be applied in core, but also possibly in both Gutenberg and core.


Describe the bug
Whenever you focus an Image Block or Gallery Block, a button element is created just before the closing body tag. Then, after blurring, the button is still there. This can be performed indefinitely times, leading to a massive amount of orphan button elements


Steps to reproduce

  1. Open up the block editor
  2. Open up dev tools and inspect the closing body tag
  3. In the editor sidebar, switch between the document and the block tab and back again
  4. Check the dev tools, observe that button(s) are appended to the dom every time the document tab is opened

Expected: No buttons are appended

Change History (6)

#1 @talldanwp
5 years ago

So I did some digging and this is happening because whenever a MediaUploader is displayed in the editor, wp.media is initialised:
https://github.com/WordPress/gutenberg/blob/a59a5d9ab761bb2e0ee37eb67aab96d7f3dcf91b/packages/media-utils/src/components/media-upload/index.js#L150

and when that initialisation happens, a new button is appended to the body element:
https://github.com/WordPress/wordpress-develop/blob/3dd39ffecea808afe86b8071331551d9d5e8cf89/src/js/media/views/uploader/window.js#L30

In #39305 @afercia mentions this button is used by another view UploaderInline, that detaches one of the buttons when the media uploader is opened, and that looks like it's still the case. But if a user never opens the media library, the button is never cleaned up.

I'm not clear on why this behaviour exists, so I feel uncomfortable modifying this code. Why does the button get appended to the body first? That's what I'd love to know if anyone has an idea!

Last edited 5 years ago by talldanwp (previous) (diff)

#2 @afercia
5 years ago

  • Focuses javascript added

@talldanwp correct. However, the core implementation makes sure to initialize wp.media just once and reuse the existing one if already initialized. Thus, in the Classic Editor there will be just one button element.

Instead, the Gutenberg implementation initializes a new wp.media each time, ending up in multiple buttons appended to the body.

<button type="button" class="browser" style="display: none;"></button>
<button type="button" class="browser" style="display: none;"></button>
<button type="button" class="browser" style="display: none;"></button>
<button type="button" class="browser" style="display: none;"></button>

Additionally, on current trunk:

  • create a post
  • add an image block and do not add an image yet: just keep the block "placeholder'
  • inspect the DOM in your browser's dev tools
  • notice two buttons appended to the body
  • add a gallery block
  • notice two more buttons are appended to the body

This makes me think that there's something that actually runs twice each time a media-related block is added. Not ideal from a performance perspective as I suspect there are several media objects initialized.

Noting the initialization issue was already reported somewhere in other GitHub issues or Trac tickets and still to be solved.

#3 @afercia
5 years ago

This is an example of the core implementation, where:

  • the media uploader is initialized only when clicking on the "choose from library" button
  • it returns early if the media frame already exist

https://core.trac.wordpress.org/browser/trunk/src/js/_enqueues/admin/custom-background.js?rev=43347&marks=86-143#L79

Instead, I guess Gutenberg initializes the uploader each time a media-related block is inserted (and does that twice?).

P.S. seems to me this is more related to the Editor component rather than Media.

Last edited 5 years ago by afercia (previous) (diff)

#4 @talldanwp
5 years ago

Hey @afercia, It'd still be good to get an understanding of why this button element is attached to the body. I think there could be two fixes, first perhaps making it so that the media uploader doesn't attach elements to the body, making this behaviour better encapsulated. Second, potentially changing the way media is initialised in the editor to avoid multiple instances.

#5 @afercia
5 years ago

HI @talldanwp
It's generated by wp.media.view.UploaderWindow. Not sure but I think it's then used by plupload. @azaozz knows more :)

#6 @glendaviesnz
2 years ago

@talldanwp I wasn't able to replicate this on WP 5.9 and GB 13.1 (https://github.com/WordPress/gutenberg/issues/9184#issuecomment-1119216257), so seems to have been resolved somehow.

Note: See TracTickets for help on using tickets.