Opened 6 years ago
Last modified 6 months ago
#45903 assigned defect (bug)
Twenty Nineteen: Some menu clicks trigger console errors
Reported by: | kjellr | Owned by: | |
---|---|---|---|
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
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:
Attachments (3)
Change History (26)
#1
@
6 years ago
- Keywords has-screenshots added
- Milestone changed from Awaiting Review to Future Release
#2
@
6 years ago
- Keywords has-patch needs-testing added; needs-patch removed
45903.patch copies over the changes from @jmlapam's GitHub PR.
#4
@
4 years 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.
#7
@
4 years 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.
4 years ago
This ticket was mentioned in Slack in #core by whyisjake. View the logs.
4 years ago
#10
@
4 years 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.
4 years ago
#12
@
4 years 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 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#15
@
4 years 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.
4 years ago
#17
@
4 years 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.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#19
@
4 years 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.
#20
@
3 years ago
As a quick fix to get rid of the flowing annoying errors, I added a conditional test for the existence of
event.target.matches
if ( event.target.matches && event.target.matches('.main-navigation > div > ul > li a') ) {
I did not test any further for potential adversarial consequences but my child theme is working like a charm now.
Thanks for moving this over, @kjellr! I can recreate both of these issues.