Make WordPress Core

Opened 5 years ago

Closed 12 months ago

Last modified 12 months ago

#46474 closed defect (bug) (fixed)

Twenty Nineteen: function matches() does not exist

Reported by: janpaulkleijn's profile janpaulkleijn Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version: 5.1
Component: Bundled Theme Keywords: has-patch commit
Focuses: Cc:

Description

In the themefiles (in the /js dir) there is a file called 'touch-keyboard-navigation.js'. This file uses the function matches() on lines 167 and 172.

It appears there is an issue with the use of the function matches() in my browser (Mozilla Firefox 65.0.2 64bit). I get the notice in the console that the function matches() does not exist.

I solved this with the use of a polyfill that is published by Mozilla: https://developer.mozilla.org/en-US/docs/Web/API/Element/matches#Polyfill

Steps to reproduce: Use the twentynineteen theme in the aforementioned Mozilla Firefox browser and click on the page (anywhere).

Attachments (2)

46474.diff (721 bytes) - added by mukesh27 5 years ago.
Patch.
#46474.patch (606 bytes) - added by rehanali 2 years ago.
Added patch

Download all attachments as: .zip

Change History (14)

#1 @janpaulkleijn
5 years ago

Steps (2) to reproduce update: To reproduce the issue, use the twentynineteen theme in the aforementioned Mozilla Firefox browser, go to the homepage and switch to another tab and then back to the twentynineteen theme.

The error thrown by Mozilla firefox:

TypeError: event.target.matches is not a function

More specs about the error thrown by Mozilla firefox:

toggleSubmenuDisplay https://test.mega-service.nl/wp-content/themes/biggee-theme/js/touch-keyboard-navigation.js?ver=1.1:277

The line of code involved (line 277):

...
if ( event.target.matches('.main-navigation > div > ul > li a') ) {
...

#2 @swissspidy
5 years ago

  • Keywords needs-patch needs-testing added; has-patch removed
  • Summary changed from function matches() does not exist in the Twenty Nineteen theme to Twenty Nineteen: Element.matches() does not exist

This is the polyfill you are referring too?

if (!Element.prototype.matches) {
  Element.prototype.matches = Element.prototype.msMatchesSelector ||
                              Element.prototype.webkitMatchesSelector;
}

#3 @janpaulkleijn
5 years ago

  • Keywords has-patch added; needs-patch needs-testing removed
  • Summary changed from Twenty Nineteen: Element.matches() does not exist to function matches() does not exist in the Twenty Nineteen theme

Forget the polyfill, I was wrong. I suppressed the issue by changing line 277 from:

...
if ( event.target.matches('.main-navigation > div > ul > li a') ) {
...

to:

...
if ( event.target != window.document && event.target.matches('.main-navigation > div > ul > li a') ) {
...

But perhaps you have a better solution?

#4 @mukesh27
5 years ago

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

@janpaulkleijn, Firefox shows an error when we focus on current page menu item, Below your solution solve that issue for for me in Firefox.

if ( event.target != window.document && event.target.matches('.main-navigation > div > ul > li a') ) {

@mukesh27
5 years ago

Patch.

#5 @mukesh27
5 years ago

  • Keywords has-patch added; needs-patch removed

#6 @poena
4 years ago

  • Component changed from Themes to Bundled Theme

#7 @SergeyBiryukov
4 years ago

  • Summary changed from function matches() does not exist in the Twenty Nineteen theme to Twenty Nineteen: function matches() does not exist

@rehanali
2 years ago

Added patch

#8 @poena
14 months ago

  • Milestone changed from Awaiting Review to 6.3

Both patches solve the JavaScript error for me, and the menus still work well.

Tested on windows 10:
Firefox; version 112.0.1
Chrome: Version 112.0.5615.121
Edge: Version 112.0.1722.48
Opera: version 97.0.4719.63
MacOS:
Firefox: version 112.0.1
Chrome: Version 112.0.5615.12
Safari 16.4
Android phone:
Firefox 112.0,
Chrome 112.0.5615.100,
Edge

Related: This issue, plus one other, is also reported at https://core.trac.wordpress.org/ticket/45903

#9 @audrasjb
12 months ago

  • Keywords commit added; needs-testing removed

I can confirm the patch solves the issue.
Self assigning for commit.

I'll add the props from the other ticket as well.

#10 @audrasjb
12 months ago

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

#11 @audrasjb
12 months ago

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

In 55970:

Twenty Nineteen: Prevent a console error related to the main navigation on Firefox.

This changeset improves a JS conditional statement to fix a console error thrown by Firefox on event.target.matches.

Props kjellr, laurelfulford, audrasjb, dimijazz, ianbelanger, pbiron, poena, McAlyster, janpaulkleijn, swissspidy, mukesh27, rehanali.
Fixes #46474.
See #45903 (fixes one of the two issues of this ticket).

#12 @audrasjb
12 months ago

In 55972:

Twenty Nineteen: Fix a JS coding standard issue found after [55970].

Follow-up to [55970].

See #46474.

Note: See TracTickets for help on using tickets.