WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 6 months ago

#40354 closed defect (bug) (fixed)

Improper use of jQuery hasClass method

Reported by: kostasx Owned by: adamsilverstein
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.4
Component: Media Keywords: good-first-bug has-patch
Focuses: javascript Cc:

Description

According to the jQuery documentation (https://api.jquery.com/hasclass/), the .hasClass() method accepts a class name without a prefixed . dot.
An improper use of the method's argument, that includes the dot, has been found in the following files:

/wp-includes/js/mediaelement/wp-mediaelement.js
Line #47

return ! $( this ).parent().hasClass( '.mejs-mediaelement' );

Should be:

return ! $( this ).parent().hasClass( 'mejs-mediaelement' );

/wp-includes/js/mediaelement/wp-mediaelement.min.js

Line #1

return!b(this).parent().hasClass(".mejs-mediaelement")

Should be:

return!b(this).parent().hasClass("mejs-mediaelement")

Attachments (1)

40354.diff (587 bytes) - added by Presskopp 8 months ago.

Download all attachments as: .zip

Change History (10)

#1 follow-up: @SergeyBiryukov
8 months ago

  • Milestone changed from Awaiting Review to 4.8
  • Version changed from 4.7.3 to 4.4

Hi @kostasx, welcome to WordPress Trac! Thanks for the report.

Introduced in [34346].

#2 in reply to: ↑ 1 @kostasx
8 months ago

Thank you @SergeyBiryukov.
It's nice to be able to contribute and give back to the community of WordPress.

#3 follow-up: @adamsilverstein
8 months ago

  • Keywords needs-patch good-first-bug added

@kostasx thanks for the report.

I was able to verify the selector doesn't work the way it is written. Did you discover this bug because of some unexpected behavior? The fix looks straightforward here, I want to make sure we are actually fixing and not breaking anything so it is helpful to be able to reproduce.

#4 in reply to: ↑ 3 @kostasx
8 months ago

Replying to adamsilverstein:

@kostasx thanks for the report.

I was able to verify the selector doesn't work the way it is written. Did you discover this bug because of some unexpected behavior? The fix looks straightforward here, I want to make sure we are actually fixing and not breaking anything so it is helpful to be able to reproduce.

Hello. Yes, I discovered the bug while working with multiple media elements (audio players) on a page. When the wrong selector was used, as is the case in the pre-patched version, an already activated media element was re-activated causing the element to start playing again. When the patch was applied, the already activated media element continued playing with any problem.

#5 @adamsilverstein
8 months ago

@kostasx great, thanks for clarifying - I thought it would cause something like this issue. Your description of the problem and the fact that you tested it and it resolved the issue gives me confidence that this is a valid fix.

#6 @adamsilverstein
8 months ago

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

#7 @Presskopp
8 months ago

the patch

@Presskopp
8 months ago

#8 @Presskopp
8 months ago

  • Keywords has-patch added; needs-patch removed

#9 @adamsilverstein
6 months ago

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

In 40659:

Media: Fix improper use of jQuery hasClass method.

The jQuery hasClass method accepts a class name without a prefix '.' (period). Remove an errant class name with a '.' in wp-mediaelement.js that broke the selector in certain circumstances.

Props kostasx, Presskopp.
Fixes #40354.

Note: See TracTickets for help on using tickets.