Image Block: Lightbox does not work

[t-hamano]

Description

When I insert an Image block with the lightbox enabled into the content, the lightbox doesn't work. Also, the Navigation block doesn't work either.

The following error is logged in the browser console:

view.js:46 Uncaught (in promise) ReferenceError: Cannot access 'state' before initialization
    at get isMenuOpen (view.js:46:21)
    at callback (signals.ts:106:15)
    at y.x (utils.ts:179:11)
    at __webpack_modules__../node_modules/@preact/signals-core/dist/signals-core.module.js.h (index.ts:567:22)
    at c (index.ts:415:18)
    at t (index.ts:61:39)
    at r (index.ts:102:3)
    at deepMerge (state.ts:397:7)
    at store (store.ts:247:12)
    at view.js:21:33

I think this issue is related to #66001. I temporarily reverted #66001 and it fixed the issue.

Step-by-step reproduction instructions

This issue is a bit unstable, refresh your browser a few times if you can't reproduce it.

• Insert an Image block.
• Apply lightbox and save the post.
• Check the post on the frontend.

Screenshots, screen recording, code snippet

89cc186e4553bad95174b37ec3c35dc0.mp4

Environment info

No response

Please confirm that you have searched existing issues in the repo.

• Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

• Yes

Please confirm which theme type you used for testing.

• Block
• Classic
• Hybrid (e.g. classic with theme.json)
• Not sure

[t-hamano]

After testing a few different combinations, this appears to occur when the page contains a Navigation block and an Image block with lightbox enabled.

[annezazu]

@artemiomorales can you take a look at this? @t-hamano what version of WP and GB are you using here?

[artemiomorales]

@annezazu I've started taking a look 👍

[artemiomorales]

I've been able to reproduce this using Gutenberg 19.6.0-rc.3, on both WordPress 6.6.2 and 6.7-RC2. It's only present when Gutenberg is enabled.

it seems this has less to do with the lightbox specifically and more to do with Interactivity API internals via #66001 as @t-hamano pointed out. Namely, I see that the error originates in the Navigation block's view.js when trying to run the store() method.

Perhaps it'd be more apt for @michalczaplinski or @DAreRodz to take a look?

[cbravobernal]

Checking the state before using it would fix it. But we should not rely on this solution and fix it by default on the store function.
I can take a look at it later.

Pinging @westonruter , as I don't know how window.scheduler?.yield works under the hood.

diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js
index 9da7ab70d8..1f63b9e2a0 100644
--- a/packages/block-library/src/navigation/view.js
+++ b/packages/block-library/src/navigation/view.js
@@ -24,26 +24,26 @@ const { state, actions } = store(
 		state: {
 			get roleAttribute() {
 				const ctx = getContext();
-				return ctx.type === 'overlay' && state.isMenuOpen
+				return ctx.type === 'overlay' && state?.isMenuOpen
 					? 'dialog'
 					: null;
 			},
 			get ariaModal() {
 				const ctx = getContext();
-				return ctx.type === 'overlay' && state.isMenuOpen
+				return ctx.type === 'overlay' && state?.isMenuOpen
 					? 'true'
 					: null;
 			},
 			get ariaLabel() {
 				const ctx = getContext();
-				return ctx.type === 'overlay' && state.isMenuOpen
+				return ctx.type === 'overlay' && state?.isMenuOpen
 					? ctx.ariaLabel
 					: null;
 			},
 			get isMenuOpen() {
 				// The menu is opened if either `click`, `hover` or `focus` is true.
 				return (
-					Object.values( state.menuOpenedBy ).filter( Boolean )
+					Object.values( state?.menuOpenedBy ).filter( Boolean )
 						.length > 0
 				);
 			},
@@ -109,7 +109,7 @@ const { state, actions } = store(
 			handleMenuKeydown( event ) {
 				const { type, firstFocusableElement, lastFocusableElement } =
 					getContext();
-				if ( state.menuOpenedBy.click ) {
+				if ( state?.menuOpenedBy.click ) {
 					// If Escape close the menu.
 					if ( event?.key === 'Escape' ) {
 						actions.closeMenu( 'click' );
@@ -160,7 +160,7 @@ const { state, actions } = store(
 
 			openMenu( menuOpenedOn = 'click' ) {
 				const { type } = getContext();
-				state.menuOpenedBy[ menuOpenedOn ] = true;
+				state?.menuOpenedBy[ menuOpenedOn ] = true;
 				if ( type === 'overlay' ) {
 					// Add a `has-modal-open` class to the <html> root.
 					document.documentElement.classList.add( 'has-modal-open' );
@@ -169,9 +169,9 @@ const { state, actions } = store(
 
 			closeMenu( menuClosedOn = 'click' ) {
 				const ctx = getContext();
-				state.menuOpenedBy[ menuClosedOn ] = false;
+				state?.menuOpenedBy[ menuClosedOn ] = false;
 				// Check if the menu is still open or not.
-				if ( ! state.isMenuOpen ) {
+				if ( ! state?.isMenuOpen ) {
 					if (
 						ctx.modal?.contains( window.document.activeElement )
 					) {
@@ -191,7 +191,7 @@ const { state, actions } = store(
 			initMenu() {
 				const ctx = getContext();
 				const { ref } = getElement();
-				if ( state.isMenuOpen ) {
+				if ( state?.isMenuOpen ) {
 					const focusableElements =
 						ref.querySelectorAll( focusableSelectors );
 					ctx.modal = ref;
@@ -202,7 +202,7 @@ const { state, actions } = store(
 			},
 			focusFirstElement() {
 				const { ref } = getElement();
-				if ( state.isMenuOpen ) {
+				if ( state?.isMenuOpen ) {
 					const focusableElements =
 						ref.querySelectorAll( focusableSelectors );
 					focusableElements?.[ 0 ]?.focus();

[westonruter]

How was scheduler.yield() identified as the source of the problem?

[t-hamano]

@westonruter I verified via git bisect that this issue first appeared in #66001. If you run git revert cd8d4dc, you will see that this problem is solved.

[cbravobernal]

Also, only happens in Chrome 😅

[westonruter]

Well, I'm guessing that Preact is perhaps itself using setTimeout() to break up a task, and so when splitTask is using setTimeout() as well, then the dependent initialization code for the state is getting set up at the right time. When splitting tasks with setTimeout() the split task is added to the end of the queue, whereas with scheduler.yield() the split task is added to the beginning of the queue. So perhaps there needs to be a single hard-coded split task using setTimeout() up-front for the sake of Preact but then within the loop which hydrates each of the blocks we can continue to leverage scheduler.yield()?

[westonruter]

It seems there is also a race condition going on as well. If I do a hard reload of the page, I only get the error with state about half the time, whereas if I do a soft reload it then occurs almost every time:

Screen.recording.2024-11-05.11.12.44.webm

[westonruter]

PR incoming.

[westonruter]

See proposed fix in #66772. It doesn't seem to be an issue with Preact at all. In fact, it seems the Interactivity API's init function was not correctly delaying itself until after the interactive blocks have initialized. This issue was masked by the fact that the hydration loop started with await splitTask() which meant it was delaying execution when splitTask() was implemented with setTimeout(). But when splitTask() is implemented with scheduler.yield() then the tail of the slit task is added to the front of the task queue instead of the end, meaning that the hydration loop continues before the modules are given a chance to initialize. So my proposed fix is just to add an initial setTimeout() before the hydration loop to force a delay to ensure the module dependencies have initialized.