WordPress.org

Make WordPress Core

Opened 7 weeks ago

Closed 12 days 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 7 weeks ago.

Download all attachments as: .zip

Change History (10)

#1 follow-up: @SergeyBiryukov
7 weeks 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
7 weeks ago

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

#3 follow-up: @adamsilverstein
7 weeks 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
7 weeks 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
7 weeks 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
7 weeks ago

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

#7 @Presskopp
7 weeks ago

the patch

@Presskopp
7 weeks ago

#8 @Presskopp
7 weeks ago

  • Keywords has-patch added; needs-patch removed

#9 @adamsilverstein
12 days 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.