Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#53619 closed defect (bug) (fixed)

Twenty Twenty-One: JS Error for Anchors that load custom modals or react hash routes

Reported by: rixeo's profile rixeo Owned by: audrasjb's profile audrasjb
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch has-testing-info commit
Focuses: javascript Cc:

Description

When using hash urls that load custom modals or react hash routes in the front-end from the primary navigation, the script expects an anchor to exist for the hash.
The error reproduced is:

Uncaught TypeError: anchor is null
    navMenu /wp-content/themes/twentytwentyone/assets/js/primary-navigation.js?ver=1.3:165

The code in specific is :

setTimeout(function () {
  var anchor = document.getElementById(event.target.hash.slice(1));
  anchor.scrollIntoView();
}, 550);

Which should be changed to check if the anchor actually exists as :

setTimeout(function () {
  var anchor = document.getElementById(event.target.hash.slice(1));
  if ( anchor ) {
     anchor.scrollIntoView();
  }
}, 550);

Attachments (1)

53619.diff (616 bytes) - added by sabernhardt 3 years ago.

Download all attachments as: .zip

Change History (16)

#1 @rixeo
3 years ago

  • Summary changed from JS Error for Anchors that load custom modals or react hash routes to Twenty Twenty-One: JS Error for Anchors that load custom modals or react hash routes

@sabernhardt
3 years ago

#2 @sabernhardt
3 years ago

  • Keywords has-patch needs-testing added

Hi and thanks for the report!

I made a patch with your proposed change.

#3 @sabernhardt
3 years ago

  • Milestone changed from Awaiting Review to 5.9

#4 @hellofromTonya
3 years ago

  • Keywords needs-testing-info added

@sabernhardt thank you for the patch. @rixeo thank you for reporting this issue.

Can either of you please provide step-by-step instructions on how to reproduce the issue? This information helps testers to confirm the problem and validate the patch resolves it.

#5 follow-up: @rixeo
3 years ago

@hellofromTonya sure :) You can add a custom nav menu that points to a custom hash. Clicking it on the frontend will trigger the error

#6 in reply to: ↑ 5 ; follow-up: @hellofromTonya
3 years ago

Replying to rixeo:

sure :) You can add a custom nav menu that points to a custom hash. Clicking it on the frontend will trigger the error

Thank you.

The term "custom hash" may be unfamiliar for contributors. I assume it's referring to an anchor that links to specific spot in a web page. Is this correct?

What could be helpful is to provide the setup instructions that guide testers to prepare for testing when a "custom hash" exists and when it doesn't exist.

#7 in reply to: ↑ 6 @rixeo
3 years ago

  • In the WordPress admin backend, navigate to Appearance > Menus
  • Select a menu that shows in the "Primary Menu" Display Location. If there is no new menu, create a new one and assign it to the "Primary Menu" under Display Location
  • Under the "Add menu items" section, expand the sub-section "Custom Links"
  • In the field URL, add something like #test
  • In the Field Link Text add something like "Sample Link"
  • Click the Button Add To Menu to add the item to your menu
  • Click Save Menu which should be on the right near the bottom
  • Visit your WordPress site frontend and click on the custom link
  • Open your browser console (https://balsamiq.com/support/faqs/browserconsole/), select the console tab
  • Now click the custom link in the navigation menu
  • Before the fix, there should be an error shown in the browser console that starts with : Uncaught TypeError: anchor is null

Replying to hellofromTonya:

Replying to rixeo:

sure :) You can add a custom nav menu that points to a custom hash. Clicking it on the frontend will trigger the error

For more help on the navigation menu, please visit https://codex.wordpress.org/WordPress_Menu_User_Guide

Thank you.

The term "custom hash" may be unfamiliar for contributors. I assume it's referring to an anchor that links to specific spot in a web page. Is this correct?

What could be helpful is to provide the setup instructions that guide testers to prepare for testing when a "custom hash" exists and when it doesn't exist.

Last edited 3 years ago by sabernhardt (previous) (diff)

#8 @Boniu91
3 years ago

  • Keywords has-testing-info added; needs-testing-info removed

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#11 @pbearne
3 years ago

tested

#12 @sabernhardt
3 years ago

  • Keywords commit added; needs-testing removed

#13 @audrasjb
3 years ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

Self-assigning for final review then commit consideration.

#14 @audrasjb
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52213:

Twenty Twenty-One: Check if anchor exists before triggering in-page navigation.

This prevents JavaScript errors when using hash in URLs without matching them to any existing anchor in the page.

Props rixeo, sabernhardt, pbearne.
Fixes #53619.

#15 @audrasjb
3 years ago

Thanks all for working on this ticket. Committed to WP 5.9.

Note: See TracTickets for help on using tickets.