Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a more specific <script> matching #23407

Merged
merged 2 commits into from
Jun 24, 2020
Merged

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Jun 24, 2020

Description

Uses a more specific matching pattern to only affect the <script> tags which we're interested in.
This should resolve the issues in #23158

This uses the full <script> produced by WP_Scripts::do_item() ignoring the Translations and before/after inline scripts, which allows for it to exclude <script% tags contained within the before/after html chunks.

The major change I see here after testing is two fold:

  1. Script tags in the JSON are not altered
  2. The sub-scripts of wp-polyfill no longer get data-handle elements, as they're not built the same way, where they previously were, eg:
( 'fetch' in window ) || document.write( '<script src="https://example.site/wp-includes/js/dist/vendor/wp-polyfill-fetch.min.js?ver=3.0.0"></scr' + 'ipt>' );

I'm unsure if this is a concern, as I don't believe scripts should be depending on individual wp-polyfill-* scripts and only the main polyfill library?

How has this been tested?

As in #23158

  • Activate the Block experiment in Gutenberg -> Experiments and Yoast at the same time.
  • Verify that the editor does not WSOD
  • Verify that the HTML source contains data-handle attributes on appropriate script tags.
  • Visual diff comparison of the source of post-new.php comparing before/after.

Types of changes

Bug Fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

dd32 added 2 commits June 24, 2020 13:10
…gs which we're interested in.

This uses the full `<script>` produced by `WP_Scripts::do_item()` ignoring the Translations and before/after inline scripts, which allows for it to exclude `<script%` tags contained within the before/after html chunks.
@dd32 dd32 added the [Feature] Block Directory Related to the Block Directory, a repository of block plugins label Jun 24, 2020
@gziolo
Copy link
Member

gziolo commented Jun 24, 2020

@Djennez can you confirm it solves the issue? We can include it in the Gutenberg plugin release later today.

@gziolo gziolo added [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. [Priority] High Used to indicate top priority items that need quick attention labels Jun 24, 2020
@gziolo gziolo added this to the Gutenberg 8.4 milestone Jun 24, 2020
@gziolo
Copy link
Member

gziolo commented Jun 24, 2020

@noisysocks, it would be great to include this fix in the plugin release today.

@tellyworth tellyworth self-requested a review June 24, 2020 05:18
Copy link
Contributor

@tellyworth tellyworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a smart solution, much quicker than the regex idea. It worked for me in testing.

@noisysocks noisysocks merged commit 2e20984 into WordPress:master Jun 24, 2020
noisysocks pushed a commit that referenced this pull request Jun 24, 2020
* Use a more specific matching pattern to only affect the `<script>` tags which we're interested in.

This uses the full `<script>` produced by `WP_Scripts::do_item()` ignoring the Translations and before/after inline scripts, which allows for it to exclude `<script%` tags contained within the before/after html chunks.

* Rename the variable
This was referenced Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Directory Related to the Block Directory, a repository of block plugins [Priority] High Used to indicate top priority items that need quick attention [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants