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

Block-Editor: Missing DOMRect polyfill in 7.8 for IE11 #21185

Closed
gwwar opened this issue Mar 26, 2020 · 5 comments · Fixed by #21188
Closed

Block-Editor: Missing DOMRect polyfill in 7.8 for IE11 #21185

gwwar opened this issue Mar 26, 2020 · 5 comments · Fixed by #21188
Assignees
Labels
Browser Issues Issues or PRs that are related to browser specific problems [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@gwwar
Copy link
Contributor

gwwar commented Mar 26, 2020

Testing Instructions

  • Using WP 5.3.2/Gutenberg plugin 7.8 and IE11 in Windows, try interacting with the block toolbar:
  • Several JS errors will fire
  • Observe that window.DOMRect is undefined

Screen Shot 2020-03-26 at 3 29 01 PM

Note that this should have been fixed by #20110
but perhaps this is not being enqueued.

cc @aduth

@gwwar
Copy link
Contributor Author

gwwar commented Mar 26, 2020

Is it intentional that we're missing the vendor folder in https://github.com/WordPress/gutenberg/releases/tag/v7.8.0 ?

@kwight
Copy link

kwight commented Mar 26, 2020

@aduth Maybe #20225 has something to do with the vendor directory not being correctly assembled?

@aduth
Copy link
Member

aduth commented Mar 26, 2020

The vendor/ folder should be present. I'm not sure why it wasn't included in the release. When I run the plugin build script on master, I see that it fails and aborts at the step of downloading vendor scripts:

Downloading remote vendor scripts... 🛵


 > vendor/ ... %   

Dissecting the build, I see it runs this command:

php bin/get-vendor-scripts.php

Running that directly yields a more useful error:

⇒ php bin/get-vendor-scripts.php

Fatal error: Uncaught Error: Class 'WP_Patterns_Registry' not found in /Users/andrew/Documents/Code/gutenberg/lib/client-assets.php:654
Stack trace:
#0 /Users/andrew/Documents/Code/gutenberg/bin/get-vendor-scripts.php(33): require_once()
#1 {main}
  thrown in /Users/andrew/Documents/Code/gutenberg/lib/client-assets.php on line 654

For context, the vendor scripts are downloaded by loading the client-assets.php file with only a few WordPress functions stubbed (source). This code is problematic because it assumes these classes to already be defined, but they are not when the client-assets.php file is loaded directly:

/*
* Register default patterns if not registered in Core already.
*/
if ( ! WP_Patterns_Registry::get_instance()->is_registered( 'text-two-columns' ) ) {
register_pattern( 'core/text-two-columns', gutenberg_load_block_pattern( 'text-two-columns' ) );
register_pattern( 'core/two-buttons', gutenberg_load_block_pattern( 'two-buttons' ) );
register_pattern( 'core/cover-abc', gutenberg_load_block_pattern( 'cover-abc' ) );
register_pattern( 'core/two-images', gutenberg_load_block_pattern( 'two-images' ) );
}

These were introduced in #21074 (cc @youknowriad , @ellatrix )

Ideally, I think this code should be in its own file, and only executed in the context of an action callback, not run directly as part of the top-level PHP code.

As an immediate fix, it may be possible to simply add a class_exists guard to avoid that code being run in the plugin build script.

My opinion is that this should be committed and cherry-picked to the release/7.8 branch for a plugin patch release. Noting below that this is technically not a regression of the current release version, though certainly a bug.

@aduth aduth added [Type] Bug An existing feature does not function as intended Browser Issues Issues or PRs that are related to browser specific problems labels Mar 26, 2020
@aduth
Copy link
Member

aduth commented Mar 26, 2020

Fix: #21188

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Mar 26, 2020
@aduth
Copy link
Member

aduth commented Mar 27, 2020

Noting that the vendor/ folder has been absent since 7.7.0 (2 major releases ago). You can see its removal here:

https://plugins.trac.wordpress.org/changeset/2258910

It was also the case that this specific DOMRect polyfill has been missing for some time (in fact, maybe never existed at all). This was technically fixed in #20225, but the current issue with WP_Patterns_Registry means the code implemented in that pull request is not currently able to run.

Also worth noting: This polyfill is provided with WordPress 5.4 (currently RC, scheduled for final release on Tuesday). Related: https://core.trac.wordpress.org/ticket/49360


Looking at this with @tellthemachines , I (we) am struggling to reproduce the exact issues reported, as far as errors or usability issues with the block toolbar.

I can confirm that the polyfill is missing (no DOMRect on the window). The full extent of the impact of this missing is not clear. I expect issues like #19979 may still be present.


#21188 or an equivalent solution is still necessary, in order to allow the plugin code to run seamlessly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Browser Issues Issues or PRs that are related to browser specific problems [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants