WordPress 5.8: Prevent white screen and errors on Widget Screen #1390

laurelfulford · 2021-06-29T20:02:35Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

With WordPress 5.8's introduction of Widget Blocks, enqueue_block_editor_assets is now also being used on the Appearance > Widgets screen, to make sure all of the block assets are loaded. In the theme we have a number of JS files we're loading into the editor using this hook that are strictly for posts/pages: featured image placements, hiding page titles and updated dates, and article subtitles. In particular, the post-meta-toggle.js is causing an error on the Widgets screen that makes it unusable. This PR adds a check for the current screen before enqueuing any of these assets.

In one related issue in the Gutenberg repo (WordPress/gutenberg#28538), there's a suggestion for possibly adding screen-specific hooks (enqueue_block_editor_assets-$screen) on top of what's there now; if that happens down the road, it should be easy to switch the current approach to use the new hook.

In WP 5.8, some front-end scripts and styles are also being loaded into the Widgets screen; the amp-fallback.js is currently firing errors because it can't find specific HTML elements. Rather than totally blocking this script (I want to try to let WordPress do it's own thing on these updated screens as much as possible!), I increased the checks for different elements so if they're not found, the code doesn't try to run.

Notes:

I was able to consistently recreate the "white screen" issue on my local test sites, but not a Jurassic Ninja one; so it's possible this won't be as catastrophic on live sites, but we still should probably not be loading unnecessary files on the widgets screen.
This PR pulls double-duty, fixing two completely unrelated issues on the Appearance > Widgets screen. If it's less confusing to handle them separately, let me know!
I'm open to feedback on either approach here; I'm especially unsure about the first one. I also tried adding checks to the post-meta-toggle.js because it's the problem file, but I can't actually see needing any of these files on the Widget screen as it works now.

Closes #1386

How to test the changes in this Pull Request:

Note: I've outlined the steps to test using two different test sites, since it saves you from having to upgrade, then downgrade, a site, but it all can be tested on one site if desired.

If you haven't already, set up a test site to run the latest WordPress 5.8 Beta (beta 4):

On a test site, install and activate the WordPress Beta Tester plugin.
Navigate to Tools > Beta Testing WordPress, and under Select the update channel you would like this website to use: pick Bleeding Edge; click 'Save'.
The screen will update on save to add new options; under Select one of the stream options below: pick Beta/RC Only and click 'Save'.
Navigate to Dashboard > Updates; you should have the option to update to version 5.8-beta4. Click that, and go grab a coffee!

Steps to test the PR:

Disable AMP, so the site is always loading the AMP fallback script.
Navigate to WP Admin > Widgets.
Note that the page doesn't load:

Apply the PR and run npm run build, then reload the page; confirm that it's now loading:

There are still a number of errors in the console, at least on my localhost; double-check that none are coming from the AMP fallback JS. (Some of the errors seem to be related to the Newspack Blocks plugin -- or at least they disappear when I disable it -- but I need to do more testing to confirm before filing an issue).
Just a precaution, make sure the per-page/post options (hide page title, featured image position, etc...) are still loading when you edit a page or post.
Confirm that the mobile menu/search/slide-out sidebar toggles still work on the front end.
Test this PR on a site running WP 5.7.2 and confirm that the per-page/post options (hide page title, featured image position, etc...), and AMP fallback JavaScript still works.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

dkoo

Works as described! I have a few non-blocking suggestions below, but feel free to ignore if they're not applicable.

dkoo · 2021-06-30T20:56:26Z

newspack-theme/js/src/amp-fallback.js

@@ -42,12 +42,8 @@
 	const mobileToggle = document.getElementsByClassName( 'mobile-menu-toggle' ),
 		body = document.getElementsByTagName( 'body' )[ 0 ],


Nit:

Suggested change

body = document.getElementsByTagName( 'body' )[ 0 ],

body = document.body,

Thanks! Updated in 4675b36.

dkoo · 2021-06-30T21:52:34Z

newspack-theme/js/src/amp-fallback.js

+		const mobileOpenButton = headerContain.getElementsByClassName( 'mobile-menu-toggle' )[ 0 ],
+			mobileCloseButton = mobileSidebar.getElementsByClassName( 'mobile-menu-toggle' )[ 0 ];


Nit: is there any reason not to use querySelector instead, since we're looking for single elements?

None at all! 😄 updated in 4675b36.

dkoo · 2021-06-30T21:52:40Z

newspack-theme/js/src/amp-fallback.js

@@ -111,6 +110,9 @@

 	// Desktop menu (AKA slide-out sidebar) fallback.
 	for ( let i = 0; i < desktopToggle.length; i++ ) {
+		const desktopOpenButton = headerContain.getElementsByClassName( 'desktop-menu-toggle' )[ 0 ],


Nit: is there any reason not to use querySelector instead, since we're looking for single elements?

Updated in 4675b36!

laurelfulford · 2021-06-30T23:12:08Z

Thanks @dkoo! I made the JS tweaks; the main issue there is that I'm pretty clumsy with vanilla JavaScript :) thanks for catching those!

## [1.39.3-alpha.1](v1.39.2...v1.39.3-alpha.1) (2021-07-06) ### Bug Fixes * ads media queries ([239ff6e](239ff6e)) * correct custom fonts in block widget preview ([#1401](#1401)) ([878b6a1](878b6a1)) * figcaption rendering bug in Safari ([#1382](#1382)) ([e274499](e274499)) * improve donate block appearance for WP 5.8 ([#1398](#1398)) ([51fc4f7](51fc4f7)) * prevent white screen and errors on Widget Screen ([#1390](#1390)) ([5731581](5731581))

matticbot · 2021-07-06T14:02:46Z

🎉 This PR is included in version 1.39.3-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## [1.39.3](v1.39.2...v1.39.3) (2021-07-06) ### Bug Fixes * ads media queries ([239ff6e](239ff6e)) * correct custom fonts in block widget preview ([#1401](#1401)) ([878b6a1](878b6a1)) * figcaption rendering bug in Safari ([#1382](#1382)) ([e274499](e274499)) * improve donate block appearance for WP 5.8 ([#1398](#1398)) ([51fc4f7](51fc4f7)) * prevent white screen and errors on Widget Screen ([#1390](#1390)) ([5731581](5731581))

matticbot · 2021-07-06T17:36:19Z

🎉 This PR is included in version 1.39.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

laurelfulford added 3 commits June 29, 2021 10:35

fix: improve check for current widget screen

df543c7

fix: add checks to AMP fallback JS to prevent errors

67126f4

fix: refine approach to loading files on widgets screen

307c713

laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Jun 29, 2021

laurelfulford requested a review from dkoo June 29, 2021 20:02

fix: re-add code comment that was accidentally deleted

9909e97

laurelfulford mentioned this pull request Jun 30, 2021

Homepage Posts: Block doesn't appear in widget area when "Load More" is enabled Automattic/newspack-blocks#781

Open

dkoo approved these changes Jun 30, 2021

View reviewed changes

github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Jun 30, 2021

fix: improve JavaScript selectors

4675b36

laurelfulford merged commit 5731581 into master Jun 30, 2021

matticbot added the released on @alpha label Jul 6, 2021

matticbot added the released label Jul 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WordPress 5.8: Prevent white screen and errors on Widget Screen #1390

WordPress 5.8: Prevent white screen and errors on Widget Screen #1390

laurelfulford commented Jun 29, 2021 •

edited

Loading

dkoo left a comment

dkoo Jun 30, 2021

laurelfulford Jun 30, 2021

dkoo Jun 30, 2021

laurelfulford Jun 30, 2021

dkoo Jun 30, 2021

laurelfulford Jun 30, 2021

laurelfulford commented Jun 30, 2021

matticbot commented Jul 6, 2021

matticbot commented Jul 6, 2021

		@@ -42,12 +42,8 @@
		const mobileToggle = document.getElementsByClassName( 'mobile-menu-toggle' ),
		body = document.getElementsByTagName( 'body' )[ 0 ],

	body = document.getElementsByTagName( 'body' )[ 0 ],
	body = document.body,

		const mobileOpenButton = headerContain.getElementsByClassName( 'mobile-menu-toggle' )[ 0 ],
		mobileCloseButton = mobileSidebar.getElementsByClassName( 'mobile-menu-toggle' )[ 0 ];

WordPress 5.8: Prevent white screen and errors on Widget Screen #1390

WordPress 5.8: Prevent white screen and errors on Widget Screen #1390

Conversation

laurelfulford commented Jun 29, 2021 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

dkoo left a comment

Choose a reason for hiding this comment

dkoo Jun 30, 2021

Choose a reason for hiding this comment

laurelfulford Jun 30, 2021

Choose a reason for hiding this comment

dkoo Jun 30, 2021

Choose a reason for hiding this comment

laurelfulford Jun 30, 2021

Choose a reason for hiding this comment

dkoo Jun 30, 2021

Choose a reason for hiding this comment

laurelfulford Jun 30, 2021

Choose a reason for hiding this comment

laurelfulford commented Jun 30, 2021

matticbot commented Jul 6, 2021

matticbot commented Jul 6, 2021

laurelfulford commented Jun 29, 2021 •

edited

Loading