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

Uncaught TypeError when adding new blocks to the Navigation Core Block in Wordpress #67712

Open
3 of 6 tasks
darnado opened this issue Dec 7, 2024 · 9 comments · May be fixed by #67718 or #67760
Open
3 of 6 tasks

Uncaught TypeError when adding new blocks to the Navigation Core Block in Wordpress #67712

darnado opened this issue Dec 7, 2024 · 9 comments · May be fixed by #67718 or #67760
Assignees
Labels
[Block] Navigation Link Affects the Navigation Link Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@darnado
Copy link

darnado commented Dec 7, 2024

Description

An "Uncaught TypeError" appears in the console when adding a block to the Navigation Core Block via the sidebar. Despite the error, the block is successfully added.

Expected Result:

  • The block should be added without any console errors.

Actual Result:

  • In Google Chrome the following console error appears: ''Uncaught TypeError: Cannot read properties of null (reading 'contains')''.
  • In Firefox the console error is slightly different: ''Uncaught TypeError: P.current is null''.
  • The block is added successfully despite the error.

Step-by-step reproduction instructions

  1. Go to Appearance → Editor in the WordPress Dashboard.
  2. Open the Design section.
  3. Navigate to Patterns → Header, and select any pattern containing the Navigation Core Block.
  4. Select the Navigation Core Block and click the "plus" button.
  5. Click "+ Add block," then select "Browse All" to open the sidebar with available blocks.
  6. Click on any block in the sidebar to add it to the Navigation Core Block.

Screenshots, screen recording, code snippet

  • Screenshot of the error in Google Chrome console.

Image

  • Screenshot of the error in Firefox console.

Image

  • Gif showcasing the error in Firefox.

Image

Environment info

  • WordPress 6.7.1.
  • Gutenberg versions: 18.6-19.3 (according to the canonical list available in this link: Versions in Wordpress)
  • Default WordPress installation.
  • Twenty Twenty-Five theme.
  • Issue reproduced on both Google Chrome and Firefox.
  • PHP version: 8.2.23

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
@darnado darnado added the [Type] Bug An existing feature does not function as intended label Dec 7, 2024
@Mamaduka Mamaduka added the [Block] Navigation Link Affects the Navigation Link Block label Dec 8, 2024
@Mamaduka
Copy link
Member

Mamaduka commented Dec 8, 2024

I can confirm the error on the Gutenberg trunk. The code in question was introduced in #61975.

// Fixes https://github.com/WordPress/gutenberg/issues/61361
// There's a chance we're closing due to the user selecting the browse all button.
// Only move focus if the focus is still within the popover ui. If it's not within
// the popover, it's because something has taken the focus from the popover, and
// we don't want to steal it back.
if (
linkUIref.current.contains(
window.document.activeElement
)
) {

cc @jeryj

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Dec 9, 2024
@jeryj
Copy link
Contributor

jeryj commented Dec 9, 2024

I think there may be a deeper bug here. I don't think this code should not be reached when adding a block from the sidebar. When I test locally, the Link UI isn't closing after I click "Browse all." It stays open and doesn't transfer focus to the sidebar.

@jeryj
Copy link
Contributor

jeryj commented Dec 9, 2024

It looks like the root issue is the Inserter is no longer auto-transferring focus to the aria-selected tab when it mounts.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 9, 2024

I think there may be a deeper bug here.

That was my guess as well. As usual, it would be better to fix the actual bug instead of its symptom (#67718). cc @yogeshbhutkar

@jeryj
Copy link
Contributor

jeryj commented Dec 9, 2024

I have to write the PR description and issue, but here's a fix for the inserter focus issue: #67754

@jeryj
Copy link
Contributor

jeryj commented Dec 9, 2024

Oddly enough, even though the link ui popover is closed from focus being transferred to the inserter in #67754, the type error is still happening. The link ui popover shouldn't be mounted at all, so I don't know why the onClose is being called...

@Mamaduka
Copy link
Member

Mamaduka commented Dec 9, 2024

It might be because the focus moves to another element.

I think this is what is happening:

  • LinkUI passes onClose to the Popover.
  • Clicking on "Browse All" moves focus and unmounts the component.
  • But onClose is called by the popover useFocusOutside hook.

@jeryj
Copy link
Contributor

jeryj commented Dec 9, 2024

Been debugging this more. I think the popover onClose is working right. My best lead so far is this line that calls the onClose again when selecting a block.

@jeryj
Copy link
Contributor

jeryj commented Dec 9, 2024

I'm not sure if it causes a new bug or misses covering a necessary scenario, but #67760 is fixing this issue at least 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
3 participants