Firefox gets "ReferenceError: Â is not defined" in unminified moxie.js

[kinggmobb]

In unminified moxie.js, there exists some weird Unicode characters on line 7391, between the BinaryReader constructor fn and the call to Basic.extend(). In a hex editor, you can see the character bytes are "C2A0", which is a random Korean Hangul character in Unicode.

In Firefox (as of the latest 117), when using Divi, this causes an exception when loading unminified moxie.js. Firefox's Js loader interprets the first byte alone as Â, and fails while loading moxie.js.

I can replicate this in FF+Divi, but the problem does not appear in Chrome, Safari, or in FF when using other builders like Classic/Gutenberg/Elementor/Beaver. Minification strips out the problem bytes.

This link, ​https://stackoverflow.com/a/1462039/1201409, suggests the error is leftover from an earlier era of encoding a nonbreaking space before UTF-8 replaced other encodings.

Regardless of how the bytes got in there (and whether FF/Divi have bugs), they shouldn't be there, and luckily, the fix is trivial: delete them.

[kinggmobb]

moxie.js error in Firefox console

[kinggmobb]

Hex editor view showing the relevant bytes

[kinggmobb]

Patch

[jorbin]

Welcome to trac @kinggmobb and thanks for the report. While you are seeing this error in WordPress, plupload is not a library that WordPress maintains.

Can you report this upstream to ​https://github.com/moxiecode/plupload and then when there is a new release WordPress can update to that?

I'm adding the close tag so that after it is reported upstream, we can close this as reported-upstream

[kinggmobb]

I was prepping to file a bug upstream, but then I took a closer look at the plupload and moxie repos.

I ran a git bisect, and turns out, the bug was silently introduced, and then inadvertently fixed, 7 years ago. WordPress is just using an ancient version of moxie.js from that time window. (There's a reference to the message I saw in ​https://github.com/moxiecode/moxie/issues/144, but I don't think anyone realized the error at the time.)

The issue was fixed right before moxie v1.5.0, in commit b9f329e1add1e19752263fe62a6b7d993bbd1906.

I think if WP upgrades to a moxie version from v1.5.0 on, it should fix the problem. Alternatively, my patch is trivial if you don't feel like upgrading moxie.js.

[desrosj]

Related #48277, #40158, #41755.

To my understanding, the AGPL is not compatible with the GPL, so simply updating Moxie and Plupload to their latest versions is not a path that can be pursued.

59329.diff​ seems like a simple and reasonable change to make to our now "adopted" version of the library.

@jorbin do you have any objection?

[q0rban]

I'm running into this same error in FireFox, when viewing site-editor.php.

[desrosj]

Apologies that this did not make it into 6.7. RC1 is due out any moment, so I'm going to punt to 6.7.1.

[desrosj]

I did some digging, and it seems like this may have been introduced around the time of #41755. Updating the Version accordingly.

Because of a few bug reports opened since 6.7 was released, the Core team is evaluating the need for a short 6.7.1 cycle (possibly next week).

This issue was not introduced in 6.7, so it now falls outside of the focus for 6.7.1 as currently defined. To help prepare for this scenario in case it's decided to move forward, I'm going to punt this to the 6.7.2 milestone.

[desrosj]

After discussing this in today's bug scrub, it's ready to commit, but going to target 6.8 instead.

59329.diff​ is good to be committed. However, the version for moxie as defined in script-loader.php needs to be changed to ensure all caches are busted.

We could go few directions here:

• Simply add -wp to the end of the current 1.3.5 version
• Since this is now an adopted fork due to upstream license changes, change the version to avoid confusion with the original upstream library. The latest is 3.1.5, so not sure that this path is realistic.
• A combination of the two. Maybe user 1.3.6-wp.

Since this is a very simple change, I think I lean towards option 1.

[azaozz]

Replying to desrosj:

Simply add -wp to the end of the current 1.3.5 version
...
A combination of the two. Maybe user 1.3.6-wp.

Think I remember having some problems with plugins when the version string of an external JS library was changed in a similar way (the plugin was expecting only digits and dots?). As this is a minor release perhaps better to "play it safe" and go with 1.3.5.1 perhaps? Also may be good to add another header to the file explaining that this file was modified/is a (de-facto) fork.

[desrosj]

Replying to azaozz:

Think I remember having some problems with plugins when the version string of an external JS library was changed in a similar way (the plugin was expecting only digits and dots?). As this is a minor release perhaps better to "play it safe" and go with 1.3.5.1 perhaps? Also may be good to add another header to the file explaining that this file was modified/is a (de-facto) fork.

Ah! Yes, I like that better. Let's go with 1.3.5.1 and introduce a header. plupload.js already has one, but doesn't explicitly say that the file is a de-facto fork.

I looked at some libraries in a similar situation (both PHP and JS), but couldn't find one with a header to that effect. It may be worth standardizing this and adding to all of our adopted libraries.

[azaozz]

Replying to desrosj:

plupload.js already has one, but doesn't explicitly say that the file is a de-facto fork.

I looked at some libraries in a similar situation (both PHP and JS), but couldn't find one with a header to that effect.

Ah, I meant adding a note to the headers that the file was modified for WP. Sorry didn't explain it well. But probably better to explain a little more and/or mention that it is forked and will be maintained by the WP developers.

It may be worth standardizing this and adding to all of our adopted libraries.

Yes, good idea! Seems some of the de-facto forked libs/files like ​twemoji.js have some inline comments. Others like ​thickbox.js don't have anything although I'm sure the original Thickbox JS was modified many times to make it work with newer jQuery, etc. (unbelievably it is from 2007, and still works?!).

Trac ticket: https://core.trac.wordpress.org/ticket/59329

[desrosj]

In 59770:

External Libraries: Remove stray Unicode character in moxie.js

This fixes a ReferenceError caused by a stray Unicode character in the unminified version of moxie.js. This has long been fixed upstream but the library cannot be wholesale updated in WordPress because of an incompatible license change.

Because of this, a new version is being tagged, 1.3.5.1, and the file header has been updated to make it more clear that the file is a maintained fork with a high level list of changes made.

Props kinggmobb, jorbin, q0rban, azaozz, desrosj, sukhendu2002.
Fixes #59329.