Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#42071 closed defect (bug) (fixed)

MediaElement upgrade causing JS errors when certain languages are in use under Admin area

Reported by: johnbillion Owned by:
Priority: normal Milestone: 4.9
Component: External Libraries Version: 4.9
Severity: major Keywords: needs-testing needs-patch
Cc: Focuses: javascript, administration

Description

MediaElement was upgraded in #39686.

This change made in [41198] is causing JavaScript errors on the post editing screen when using the admin area in a language with a three-letter locale code, such as Moroccan Arabic.

Uncaught TypeError: Language code must have format `xx` or `xx-xx`
    at Object.s.language (mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12)
    at Object.o.5.15 (mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12)
    at i (mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12)
    at e (mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12)
    at mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12
s.language @ mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12
o.5.15 @ mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12
i @ mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12
e @ mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12
(anonymous) @ mediaelement-and-player.min.js?ver=4.2.5-74e01a40:12
wp-mediaelement.js?ver=4.9-alpha-40870-src:51
Uncaught TypeError: $(...).not(...).filter(...).mediaelementplayer is not a function
    at HTMLDocument.initialize (wp-mediaelement.js?ver=4.9-alpha-40870-src:51)
    at i (jquery.js?ver=1.12.4:2)
    at Object.fireWith [as resolveWith] (jquery.js?ver=1.12.4:2)
    at Function.ready (jquery.js?ver=1.12.4:2)
    at HTMLDocument.K (jquery.js?ver=1.12.4:2)
initialize @ wp-mediaelement.js?ver=4.9-alpha-40870-src:51
i @ jquery.js?ver=1.12.4:2
fireWith @ jquery.js?ver=1.12.4:2
ready @ jquery.js?ver=1.12.4:2
K @ jquery.js?ver=1.12.4:2
wp-auth-check.js?ver=4.9-alpha-40870-src:101
Uncaught TypeError: Cannot read property 'hasClass' of undefined
    at HTMLDocument.<anonymous> (wp-auth-check.js?ver=4.9-alpha-40870-src:101)
    at HTMLDocument.dispatch (jquery.js?ver=1.12.4:3)
    at HTMLDocument.r.handle (jquery.js?ver=1.12.4:3)
    at Object.trigger (jquery.js?ver=1.12.4:3)
    at Object.jQuery.event.trigger (jquery-migrate.js?ver=1.4.1:633)
    at HTMLDocument.<anonymous> (jquery.js?ver=1.12.4:3)
    at Function.each (jquery.js?ver=1.12.4:2)
    at jQuery.fn.init.each (jquery.js?ver=1.12.4:2)
    at jQuery.fn.init.trigger (jquery.js?ver=1.12.4:3)
    at Object.<anonymous> (heartbeat.js?ver=4.9-alpha-40870-src:400)

Seen in Chrome 61 on macOS. Git bisected back to [41198].

Attachments (1)

42071.diff (200.0 KB ) - added by joemcgill 9 years ago.

Download all attachments as: .zip

Change History (24)

#1 @johnbillion
9 years ago

  • Owner set to rafa8626
  • Status newassigned

#2 @rafa8626
9 years ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.


9 years ago

This ticket was mentioned in Slack in #core-media by rafa8626. View the logs.


9 years ago

#5 @westonruter
9 years ago

  • Owner changed from rafa8626 to johnbillion
  • Status assignedreviewing

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


9 years ago

#7 @SergeyBiryukov
9 years ago

Appears to be fixed upstream in ad2286d, so should be resolved by updating ME.js to the latest version.

@joemcgill
9 years ago

#8 @joemcgill
9 years ago

  • Keywords has-patch added; needs-patch removed

42071.diff Upgrades MEJS to version 4.2.6, based on the PR linked from @rafa8626 above. @SergeyBiryukov or @johnbillion, can you test and confirm that this fixes the issue?

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


9 years ago

#10 @johnbillion
9 years ago

  • Owner johnbillion removed

Thanks for your work on this.

I've tested the unminified version of 42071.diff (related: #42229) and it's better but I'm still seeing an issues. The Kabyle language has a language code of kab-DZ which doesn't match the regex.

I'll carry on testing to see if there are any other language patterns that don't match.

#11 @johnbillion
9 years ago

  • Keywords needs-patch added; has-patch removed

I wonder if the mejsL10n.language property should be populated with get_locale() instead of get_bloginfo( 'language' ). which results in the short locale name being returned (kab instead of kab-DZ). No idea what impact this will have.

Last edited 9 years ago by johnbillion (previous) (diff)

#12 @rafa8626
9 years ago

  • Summary MediaElement upgrade causing JS errors when certain languages are in useMediaElement upgrade causing JS errors when certain languages are in use u

Thanks for the heads up. I’m not sure about th last comment but for what I can tell it’s easier if I enable the match for the pattern you described before; it won’t take that long and I think that’s the last pattern to match. Otherwise, it will truly be non-standard. I have a PR that it’s being tested and with more urgent fixes so I’ll be putting this fix there and what with you the URL so you can test it out

#13 @johnbillion
9 years ago

In fact I think it should be switched to get_locale(). The intention of get_bloginfo('language') is for the output to be used in the HTML lang attribute.

#14 @rafa8626
9 years ago

If that’s the case that’s a different issue, but I don’t know who’s in charge of that area to be honest. This ticket now has to be assigned to someone with more knowledge on that area than myself

#15 @rafa8626
9 years ago

  • Summary MediaElement upgrade causing JS errors when certain languages are in use uMediaElement upgrade causing JS errors when certain languages are in use under Admin area

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


9 years ago

#17 @westonruter
9 years ago

@rafa8626 How does this relate to #42189?

#18 @rafa8626
9 years ago

@westonruter The PR mentioned in https://core.trac.wordpress.org/ticket/42189 included this one as well; that's the only real link between them. As far as MEJs is concerned, I just expanded the regex for the situation @johnbillion mentioned yesterday and was included with other things needed by @bradyvercher in the other ticket

#19 @westonruter
9 years ago

@rafa8626 I committed the patch on #42189 so I think the patch on this one now needs to be updated?

#20 @westonruter
9 years ago

  • Resolutionfixed
  • Status reviewingclosed

@johnbillion this should be fixed by [41877], so please re-open if the issue persists for you.

#21 @erich_k4wp
9 years ago

  • Resolution fixed
  • Status closedreopened

Also JavaScript errors in a language with a locale code such as de_DE-formal

if (!/^[a-z]{2,3}((\-|_)[a-z]{2})?$/i.test(args[0])) {
  throw new TypeError('Language code must have format 2-3 letters and. optionally, hyphen, underscore followed by 2 more letters');
}

#22 @ocean90
9 years ago

  • Resolutionfixed
  • Status reopenedclosed

Hello @erich_k4wp, thanks for the report. This ticket was closed on a completed milestone. Can you please open a new ticket and mention this ticket, #42071? Thanks!

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


9 years ago

Note: See TracTickets for help on using tickets.