WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 6 days ago

#45903 assigned defect (bug)

Twenty Nineteen: Some menu clicks trigger console errors

Reported by: kjellr Owned by: audrasjb
Milestone: Future Release Priority: normal
Severity: normal Version: 5.0
Component: Bundled Theme Keywords: has-screenshots has-patch needs-refresh
Focuses: javascript Cc:

Description

There are two scenarios where the Twenty Nineteen menu javascript triggers console errors:


First, on desktop screens, clicking the main navigation arrows produce the following error:

TypeError: event.target.matches is not a function[Learn More] 4 touch-keyboard-navigation.js:285:9
	toggleSubmenuDisplay/< ../wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js:285:9

https://cldup.com/PqsNUSgudV.gif

Originally reported by @joyously in: https://github.com/WordPress/twentynineteen/issues/596


Second, on touch-enabled screens, tapping menu items that have empty (#) href attributes result in the following console errors:

TypeError: event.target.nextSibling is null[Learn More] touch-keyboard-navigation.js:223:5 
	toggleSubmenuDisplay/<	../wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js:223:5
	sendTouchEvent	resource://devtools/server/actors/emulation/touch-simulator.js:301:5
	handleEvent	resource://devtools/server/actors/emulation/touch-simulator.js:212:7
TypeError: event.target.matches is not a function[Learn More] touch-keyboard-navigation.js:277:9
	toggleSubmenuDisplay/<	http://core.test/wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js:277:9

Originally reported in: https://github.com/WordPress/twentynineteen/issues/726


It looks like a possible fix for one of these was explored by @jmau in the following PR, but it has not yet been tested:

https://github.com/WordPress/twentynineteen/pull/753/files

Attachments (3)

45903.patch (2.0 KB) - added by laurelfulford 2 years ago.
45903.1.diff (1.9 KB) - added by audrasjb 9 months ago.
Patch refresh
45903.2.diff (1.9 KB) - added by audrasjb 9 months ago.
Coding standards fixes

Download all attachments as: .zip

Change History (22)

#1 @laurelfulford
2 years ago

  • Keywords has-screenshots added
  • Milestone changed from Awaiting Review to Future Release

Thanks for moving this over, @kjellr! I can recreate both of these issues.

@laurelfulford
2 years ago

#2 @laurelfulford
2 years ago

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

45903.patch copies over the changes from @jmlapam's GitHub PR.

#3 @ianbelanger
12 months ago

  • Milestone changed from Future Release to 5.5
  • Version changed from 5.0.2 to 5.0

@audrasjb
9 months ago

Patch refresh

#4 @audrasjb
9 months ago

  • Keywords needs-testing removed

In 45903.1.diff, I refreshed the existing patch and I moved the pre-return checks after vars declarations.

Tested on my side: no error is showing.

#5 @audrasjb
9 months ago

  • Owner set to ianbelanger
  • Status changed from new to assigned

@audrasjb
9 months ago

Coding standards fixes

#6 @dimijazz
9 months ago

@audrasjb I tried your code but still get the issue please have a look at screenshot.
I get the error if I leave the window on firefox and IE

https://www.dropbox.com/s/3ugx9nvcknh6l2f/Screen%20Shot%202020-06-17%20at%204.43.09%20PM.png

#7 @ianbelanger
9 months ago

  • Keywords needs-refresh added
  • Owner changed from ianbelanger to audrasjb

I can confirm that the most recent patch does not fix the issue. I don't get any errors in Chrome, but do in Firefox and IE.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 months ago

This ticket was mentioned in Slack in #core by whyisjake. View the logs.


7 months ago

#10 @pbiron
7 months ago

With 45903.2.diff applied, I can confirm that I'm still getting the console errors in FF 78.0.2 (Windows) and IE 11.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


7 months ago

#12 @davidbaumwald
7 months ago

  • Milestone changed from 5.5 to 5.6

With 5.5 RC1 releasing tomorrow, this is being moved to 5.6. If any maintainer or committer feel this can be resolved in time or wishes to assume ownership during a specific cycle, feel free to update the milestone.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 months ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 months ago

#15 @hellofromTonya
4 months ago

  • Milestone changed from 5.6 to 5.7

5.6 is deep into the beta cycle with Beta 4 next week. This ticket hasn't had activity in months. Punting to 5.7.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 weeks ago

#17 @poena
12 days ago

I spent a significant amount of time trying to troubleshoot this but I was not able to solve it.

The file twentynineteen\js\touch-keyboard-navigation.js is missing essential documentation for
the choices made regarding the focus event listener for the menu links.
It seems to be triggered on any focus, not only when focus is on elements inside the main menu.

Last edited 12 days ago by poena (previous) (diff)

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


6 days ago

#19 @hellofromTonya
6 days ago

  • Milestone changed from 5.7 to Future Release

With 5.7 RC1 landing tomorrow and no solution yet, punting this ticket to Future Release.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

Note: See TracTickets for help on using tickets.