WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 18 months ago

#48034 closed defect (bug) (reported-upstream)

Video embeds fail if embed block is converted to a reusable block.

Reported by: donmhico Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Embeds Keywords: has-patch
Focuses: Cc:

Description

This issue is from Gutenberg's GitHub repo -

I'll just copy the main issue here - https://github.com/WordPress/gutenberg/issues/4483

Credits to niranjan-uma-shankar for raising the issue.

Embedding youtube videos works well. However, if you convert the block to a reusable block, the youtube player no longer gets embedded in the post. In the editor, the embed continues to show the player, though the play button doesn't work anymore.

Attachments (1)

48034.diff (575 bytes) - added by donmhico 2 years ago.

Download all attachments as: .zip

Change History (4)

@donmhico
2 years ago

#1 @donmhico
2 years ago

  • Keywords has-patch added

The issue seems to be caused by the mis-order of when re-usable blocks are parsed.
<!-- wp:block {"ref":XXX} /--> - which handles reusable blocks are parsed during do_block(). But at this time the filter that gets the autoembed meta is already finished and will no longer be called.

https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-embed.php#L39
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/default-filters.php#L172

The solution in the attached patch, 48034.diff, is to invoke do_block() earlier so that <!-- wp:block {"ref":XXX} /--> is already converted to the actual block content before it is passed to the autoembed filter.

I'm not entirely sure though if this will cause any backward compatibility issue. Feedback and more tests are highly appreciated.

#2 @noisysocks
18 months ago

  • Keywords 2nd-opinion commit added
  • Milestone changed from Awaiting Review to 5.5

Thanks for investigating this @donmhico! I was able to reproduce the bug using the steps in GB4483 and have confirmed that 48034.diff fixes it.

I'm not sure whether changing when this filter is executed will cause any unexpected problems. I suspect the only way we'll be able to tell for sure is by committing this and seeing if any regressions are found during the 5.5 beta period. I'd appreciate a second opinion here, though.

#3 @noisysocks
18 months ago

  • Keywords 2nd-opinion commit removed
  • Milestone 5.5 deleted
  • Resolution set to reported-upstream
  • Status changed from new to closed

Change of plan here. As mentioned in this comment, changing the order of when the do_blocks will most definitely break plugins and themes. Instead let's fix this by making the Embed block a dynamic block. I'm re-opening GB14608 to track this work.

Note: See TracTickets for help on using tickets.