HTML-Embed code JavaScript error.

[nendeb55]

I copy a cord of HTML-Embed from a page of Embed and input in a post of WordPress4.3.
JavaScript code && is converted to the && and get an error.

I suggest that I change & to influence in wptexturize to the cord not to use in wp-embed.js.

[swissspidy]

Hey there

Did you paste the embed code in the visual editor or the text editor?

Also, you need to make sure to have the unfiltered_html capability (usually admins & editors have these on single sites).

[nendeb55]

It is a text editor.
In administrative posts.

SyntaxError: illegal character
|h)&&d.getAttribute("security")&&(a=d.clone

[pento]

This is caused by convert_chars() and wptexturize() indiscriminately converting & to &, which it shouldn't be doing to & characters within <script> tags.

[pento]

34698.diff​ is a first pass at fixing this.

I removed convert_chars() from the the_content filter, as it seems to be made entirely redundant by wptexturize().

Moving the & replacement to inside the loop fixes this bug, but it causes some unit tests to fail:

1) Tests_Formatting_WPTexturize::test_tag_avoidance with data set #4 ('[ photos by <a href="http://example.com/?a[]=1&a[]=2"> this guy </a> ]', '[ photos by <a href="http://example.com/?a[]=1&#038;a[]=2"> this guy </a> ]')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'[ photos by <a href="http://example.com/?a[]=1&#038;a[]=2"> this guy </a> ]'
+'[ photos by <a href="http://example.com/?a[]=1&a[]=2"> this guy </a> ]'

/srv/www/wordpress-develop/tests/phpunit/tests/formatting/WPTexturize.php:1248

2) Tests_Formatting_WPTexturize::test_tag_avoidance with data set #5 ('[photos by <a href="http://example.com/?a[]=1&a[]=2"> this guy </a>]', '[photos by <a href="http://example.com/?a[]=1&#038;a[]=2"> this guy </a>]')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'[photos by <a href="http://example.com/?a[]=1&#038;a[]=2"> this guy </a>]'
+'[photos by <a href="http://example.com/?a[]=1&a[]=2"> this guy </a>]'

/srv/www/wordpress-develop/tests/phpunit/tests/formatting/WPTexturize.php:1248

@miqrogroove, could you please lend your expertise to this?

[nendeb55]

Thanks for the patch.

I will be reliable in this in future.
But I cannot cope in WordPress of the version that is old when it is this.
I suggest that I change a cord in wp-embed.js,wp-embed.min.js and get_post_embed_html().

Exsample in wp-embed.js

/* Only continue if link hostname matches iframe's hostname. */
if ( targetURL.host === sourceURL.host && document.activeElement === source ) {
	window.top.location.href = data.value;
}

To

/* Only continue if link hostname matches iframe's hostname. */
if ( targetURL.host === sourceURL.host ) {
	if (document.activeElement === source ) {
		window.top.location.href = data.value;
	}
}

&& It has been used three locations

[pento]

Thanks for the feedback, @nendeb55!

You make a good point, that this can still cause issues in older WordPress versions. I think that removing those instances of && that you point out would be a good solution.

[pento]

34698.2.diff​ should do the trick.

[peterwilsoncc]

In 34698.3.diff​ I've moved shuffled the for loop to avoid the nested if statements.

[nendeb55]

Thanks for These patch.

Finally, please create a JavaScript-code also patches in the wp-embed.min.js and embed-functions.php of get_post_embed_html ().

[peterwilsoncc]

Replying to nendeb55:

Thanks for These patch.

Finally, please create a JavaScript-code also patches in the wp-embed.min.js and embed-functions.php of get_post_embed_html ().

The .min file and the function are created automatically during the build process from the file in the patch by grunt. Only the uncompressed file needs to be changed in this repo.

I'm looking over what uglify.js is doing during the compression process to ensure it doesn't recreate all the && that we are removing here.

[nendeb55]

Thanks.

Does get_post_embed_html(), too?

[peterwilsoncc]

Replying to nendeb55:

Does get_post_embed_html(), too?

It does, yes.

~

I'm stuck on ensuring uglify doesn't re-introduce the && back in to the compressed code.

Most reliable method going forward seems to be to stop uglify optimising the if/for loops in this file. Reversing the logic worked in a couple of instances (!(x!=y||z!=y)) but it's not as future friendly as a custom build.

Any suggestions from the build tools team would be grand :)

[nendeb55]

Thank you for plain explanation.

[pento]

In 35708:

Embeds: Remove & characters from the inline embed JS.

Older versions of WordPress will convert those & characters to &, which makes for some non-functional JS. If folks are running an older release, let's not make their lives more difficult than it already is.

Props pento, peterwilsoncc.

See #34698.

[pento]

That fixes one half of the problem, still need to figure out what the deal is with 34698.diff​.

[pento]

On further reflection, the & within the href should be (and is) handled by KSES. The existing behaviour is incorrect.

[pento]

In 35709:

Texturize: Only convert & to & within text nodes.

Previously, & would be converted everywhere, which caused problems when it was converted within a <script>, for example.

convert_chars() is now removed from the the_content filter, as it was doing the same job as wptexturize().

KSES correctly handles converting & within HTML attributes, so there's no need for wptexturize() and convert_chars() to do the same job.

Fixes #34698.

[miqrogroove]

Is there still time to review this before 4.4? Or can this be reverted? Just wondering why we are changing the output of href values without some extra consideration.

[miqrogroove]

Needs explanations for why this is marked as a trunk defect, why the patch is committed late in the cycle, and why the unit tests are being modified.

[pento]

It's a defect because it breaks &s inside a <script> tag.

There are a few scenarios I considered before committing this change:

• A user without unfiltered_html posts content with an & in it. In an HTML attribute, it will be converted to & by KSES, in a text node it, it will be converted to  by texturize, or if it's inside a <script> tag, the tag is going to be removed.
• A user with unfiltered_html posts content with an & in it. In an HTML attribute, KSES didn't touch it, as it shouldn't. Texturize will no longer convert it to , which in the case of an href attribute, ​was incorrect, anyway. In a text node, it will still be correctly converted to  by texturize.
• The HTML came from an embed. In a text node, it will still be converted to . In a <script> tag, it will now no longer be converted.

I'm happy to make tweaks if there are other scenarios you think should be considered, but the change to <script> and href behaviour should stay.

[pento]

My mistake, I don't know why I was testing with  instead of & in the href, the latter works correctly.

In which case, I'm not particularly fussed about what happens to an & inside an attribute for unfiltered_html and embed output (all browsers recognise an un-encoded & in an href) - the <script> change just needs to stay.

[nendeb55]

HTML-Embed Code

Error has come out in a compressed JavaScript in the get_post_embed_html().
But it works fine if you try in SCRIPT_DEBUG.
It works correctly when further compress the wp-embed.js in another way.

Is there a problem in the compression stroke of JavaScript?

FireBug

TypeError: j.style is undefined
...ded-content");for(c=0;c<j.length;c++)j.style.display="none";for(c=0;c<i.length;c...

[nendeb55]

Error I found that to come out only when you enable the SyntaxHighlighter Evolved Plugin.
SyntaxHighlighter Evolved Plugin had been rewritten in this way the script.

for(c=0;c<j.length;c++)j[c].style.display="none";for(c=0;c<i.length;c++)if(d=i[c],d.style.display="

To

for(c=0;c<j.length;c++)j.style.display="none";for(c=0;c<i.length;c++)if(d=i,d.style.display="",

[peterwilsoncc]

Replying to nendeb55:

Error I found that to come out only when you enable the SyntaxHighlighter Evolved Plugin.
SyntaxHighlighter Evolved Plugin had been rewritten in this way the script.

I'm unclear, could you please clarify:

• does the error occur if the plugin is disabled either with/without SCRIPT_DEBUG?
• is the code edit your making to the embed JavaScript or the plugin's JavaScript?

[pento]

@viper007bond - Looks like this change may've had an effect on SyntaxHighlighter Evolved.

[nendeb55]

Apparently SyntaxHighlighter Evolved Plugin seems to react to the short code [c].
It was working properly If you remove the c of syntaxhighlighter_brushes definition of syntaxhighlighter.php.

..for(c=0;c<j.length;c++)j[c].style.display="none";for(c=0;c<i.length;c++)if(d=i[c],d.style.display="..

To

..for(c=0;c<j.length;c++)j.style.display="none";for(c=0;c<i.length;c++)if(d=i,d.style.display=""..

[Viper007Bond]

My plugin stupidly registers a bunch of shortcodes, the idea being you could just do [php]code here[/php] instead of [code lang="php"]code here[/code]. I've since come to regret this.

Unfortunately if I reverse this decision, it could easily break past usages of the shortcode. Perhaps I can play with filter orders instead? Currently my plugin parses its shortcodes at priority 7 (not the default shortcode priority).

[nendeb55]

Hi Viper007Bond

Thank you for your comments.
Cause is understood as an individual basis.

[dd32]

Replying to Viper007Bond:

My plugin stupidly registers a bunch of shortcodes, the idea being you could just do [php]code here[/php] instead of [code lang="php"]code here[/code]. I've since come to regret this.

Unfortunately if I reverse this decision, it could easily break past usages of the shortcode. Perhaps I can play with filter orders instead? Currently my plugin parses its shortcodes at priority 7 (not the default shortcode priority).

It sounds like you're parsing shortcodes inside of <script> tags, which you probably shouldn't do :)

[pento]

Closing this ticket now, no-one has anything to add here or in the Slack discussions above.

If any new bugs crop up related to embeds or texturize behaviour, please open a new ticket.

[miqrogroove]

I still need to voice my strong objection to allowing this change in a Release Candidate without more testing and proper unit test coverage. This should be a 4.5-early ticket. Please either revert or remove my name as owner.

[Viper007Bond]

Replying to dd32:

It sounds like you're parsing shortcodes inside of <script> tags, which you probably shouldn't do :)

Well then WordPress is to blame as I'm using core functions, just with only my shortcodes temporarily registered (I restore the normal list when I'm done).

[peterwilsoncc]

Reopening as the JS file requires a comment about avoiding the & character in future edits.

[peterwilsoncc]

In 34698.4.diff​ some docs to warn against using the ampersand character in the embed JavaScript.

[nacin]

In 35762:

Embeds: Enforce, via unit tests, the no-ampersand rule for wp-embed.js.

fixes #34698.

[nacin]

My commit, though insane, was reviewed by @helen, @markjaquith, and @jorbin.

Thanks for the patch here, @peterwilsoncc. I noticed this same thing a few days ago and had been planning to write a test to validate the lack of ampersands. It's important to not only document decisions, but also verify them where we can. :-)

[nendeb55]

WordPress4.4 the release version of wp-embed-template.js
Place I was asked to fix the &, it seems to go back to the beta-version.

[nendeb55]

For & and wp-embed-template.js was my misunderstanding.

[johnbillion]

In 40526:

Build/Test Tools: Don't trigger a skipped test when the built version of wp-embed.min.js isn't present.

The grunt command assertion prior to the check for the existence of the file is enough to safeguard this test.

See #34698, #40533

[pento]

#32921 was marked as a duplicate.

[westonruter]

In #58974 I propose that we can now eliminate this back-compat for WP≤4.3 given that all those versions of WordPress only account for 1.43% combined of the market share.