Make WordPress Core

Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#44306 closed defect (bug) (fixed)

wp-embed-template.js Sends a message earlier than the wp-embed.js listener will be installed

Reported by: cawa-93's profile Cawa-93 Owned by: westonruter's profile westonruter
Milestone: 5.9 Priority: normal
Severity: normal Version: 4.4
Component: Embeds Keywords: has-patch
Focuses: javascript Cc:

Description

I use google chrome v67.0.3396.62

In the content of the page is inserted a link to another page of the same site.

This link is replaced by the iframe.
However, this frame is loaded before downloading the listener's wp-embed. Because of this, the frame does not update its height and does not replace the blockquote element

I enabled debugging and added the output to the console to monitor the execution order. Here is the result:

14:36:01.651 (index):758 sendEmbedMessage(height, 348)
14:36:02.134 wp-embed.js?ver=4.9.6:121 addEventListener(message)
14:36:02.588 wp-embed.js?ver=4.9.6:89 onLoad()
14:36:04.889 wp-embed.js?ver=4.9.6:89 onLoad()

The widget only appears if you resize the page after loading the page. Here is the log

14:36:08.091 (index):758 sendEmbedMessage(height, 250)
14:36:08.092 wp-embed.js?ver=4.9.6:31  receiveEmbedMessage data: {message: "height", value: 250, secret: "Fr0AAXkEeU"}
14:36:08.240 (index):758 sendEmbedMessage(height, 250)
14:36:08.240 wp-embed.js?ver=4.9.6:31  receiveEmbedMessage data: {message: "height", value: 250, secret: "Fr0AAXkEeU"}

Attachments (1)

slow-post-embeds.php (1.1 KB) - added by westonruter 3 years ago.
Demonstration showing how a slow-loading wp-embed.js can cause post embeds to fail to get sized

Download all attachments as: .zip

Change History (9)

#1 @Cawa-93
7 years ago

If, however, I pre-register the script in the header and not in the footer, then the frame of the widget is displayed correctly.

<?php
add_action('wp_enqueue_scripts','embed_on_head',8);
function embed_on_head() {
        wp_enqueue_script('wp-embed', includes_url( '/js/wp-embed.min.js' ), array(), false, false);
}

Here is the log:

14:52:18.132 wp-embed.js?ver=4.9.6:121 addEventListener(message)
14:52:19.536 (index):758 sendEmbedMessage(height, 299)
14:52:19.574 wp-embed.js?ver=4.9.6:31  receiveEmbedMessage data: {message: "height", value: 299, secret: "Fr0AAXkEeU"}
14:52:21.533 wp-embed.js?ver=4.9.6:89 onLoad()
14:52:22.409 (index):758 sendEmbedMessage(height, 299)
14:52:22.510 wp-embed.js?ver=4.9.6:31  receiveEmbedMessage data: {message: "height", value: 299, secret: "Fr0AAXkEeU"}
14:52:23.259 wp-embed.js?ver=4.9.6:89 onLoad()
Last edited 7 years ago by Cawa-93 (previous) (diff)

#2 @westonruter
3 years ago

While I haven't seen this issue myself, I have solution that should prevent the issue from happening.

#3 @swissspidy
3 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from major to normal

Copying over from #44632:

To solve the race condition, I think wp-embed-template.js should

  • not send the message on load if the secret is missing
  • send the message as soon as the secret has been added (which means wp-embed.js has been loaded)

@westonruter
3 years ago

Demonstration showing how a slow-loading wp-embed.js can cause post embeds to fail to get sized

#4 @westonruter
3 years ago

  • Milestone changed from Future Release to 5.9
  • Owner set to westonruter
  • Status changed from new to accepted
  • Version changed from 4.9.6 to 4.4

#5 @westonruter
3 years ago

I have the fix for this included in the pull request for #44632: https://github.com/WordPress/wordpress-develop/pull/1745

#6 @westonruter
3 years ago

  • Keywords has-patch added

Ready for review per comment on #44632.

#7 @westonruter
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 52132:

Embeds: Conditionally enqueue wp-embed only if needed and send ready message in case script loads after post embed windows.

  • Prevent loading wp-embed script unconditionally on every page in favor of conditionally enqueueing when a post embed is detected. The wp-embed script is also explicitly marked as being in the footer group. Sites which currently disable post embed scripts from being enqueued via remove_action( 'wp_head', 'wp_oembed_add_host_js' ) will continue to do so.
  • Send a ready message from the host page to each post embed window in case the iframe loads before the wp-embed script does. When the ready message is received by the post embed window, it sends the same height message as it sends when it loads.
  • Eliminate use of grunt-include to inject emoji script and the post embed script. Instead obtain the script contents via file_get_contents() (as is done elsewhere in core) and utilize wp_print_inline_script_tag()/wp_get_inline_script_tag() to construct out the script. This simplifies the logic and allows the running of src without SCRIPT_DEBUG enabled.
  • For the embed code that users are provided to copy for embedding outside of WP, add the secret on the blockquote and iframe. This ensures the blockquote will be hidden when the iframe loads. The embed code in question is accessed here via get_post_embed_html().

Props westonruter, swissspidy, pento, flixos90, ocean90.
Fixes #44632, #44306.

#8 @SergeyBiryukov
3 years ago

In 52325:

Build/Test Tools: Remove the replace:emoji-banner-text Grunt task.

The task was previously used to ensure that /*! This file is auto-generated */ comment is not included on front end as part of the inline emoji detection script.

As the wp-emoji-loader.js script is now included via file_get_contents() and wp_print_inline_script_tag() instead of grunt-include to simplify the logic, the task does not find anything to replace and is no longer necessary.

Additionally, include a line break before the wp-emoji-loader.js script content for better line wrapping.

Follow-up to [48096], [50651], [52132].

See #44632, #44306, #53363.

Note: See TracTickets for help on using tickets.