decoding="async" breaks my site

[rodricus]

Like the title says: decoding="async" attribute in my img elements breaks my site. WordPress 6.1 brings this attribute to my img elements. It generates a javascript error: Uncaught SyntaxError: Unexpected token 'async'

The javascript functionality breaks on my website. This needs to be fixed as soon as possible. You can see the problem here: ​https://bouma.diepcreative.nl/

[rodricus]

The problem was residing in my Slick Carousel initialisation:

$('.section-service-slider .services').slick({

autoplay: false,
speed: 500,
infinite: true,
slidesToShow: 6,
slidesToScroll: 1,
prevArrow: "<button type='button' class='navigation-arrow slick-prev'><img decoding='async' src='" + arrow_url + "' /></button>",
nextArrow: "<button type='button' class='navigation-arrow slick-next'><img decoding='async' src='" + arrow_url + "' /></button>",
dots: false,
appendDots: false,
accessibility: false,
mobileFirst: false,

WordPress somehow injected " decoding='async' " in the img element. If you change that to explicitly " decoding='sync' ", then the javascript-errors are resolved and the site-page works again.

[TimothyBlynJacobs]

Hi @rodricus,

Thanks for the ticket!

Is that JavaScript code in your post content? Or is it coming from somewhere else like your theme or a widget?

[rodricus]

Hello @TimothyBlynJacobs, sorry for being unclear. Yes that's javascript-code. It's the initialisation code for the Slick Slide plugin.

[TimothyBlynJacobs]

Thanks @rodricus. What I mean, is are you inserting that JavaScript code into one of your posts or pages? As in the actual content of the page with perhaps the HTML Block?

[joelmadigan]

I'd like to chime in with an instance of the same issue in a slightly different context.

In my case, NinjaForms apparently uses JSON+JavaScript to load their forms and the method used by WordPress to inject the decode="async" attribute doesn't take the image possibly being in a double-quoted string into account.

{
  "objectType": "Field",
  "objectDomain": "fields",
  "editActive": false,
  "order": 14,
  "idAttribute": "id",
  "label": "HTML",
  "type": "html",
  "default": "<p><img decoding="async " style=\"width: 604px;\" src=\"https:\/\/www.lewislandscape.com\/wp-content\/uploads\/2022\/05\/CheckImage.png\"><br><\/p>",
  "container_class": "",
  "element_class": "",
  "key": "html_1652099495998",
  "drawerDisabled": false,
  "id": 28,
  "beforeField": "",
  "afterField": "",
  "value": "<p><img decoding="async " style=\"width: 604px;\" src=\"https:\/\/www.lewislandscape.com\/wp-content\/uploads\/2022\/05\/CheckImage.png\"><br><\/p>",
  "label_pos": "above",
  "parentType": "html",
  "element_templates": ["html", "input"],
  "old_classname": "",
  "wrap_template": "wrap"
},

As we can see in this snippet, "default" and "value" have the string terminated early causing the JavaScript parser to abend with Unexpected Keyword: async.

I fixed it with

<?php
add_filter('wp_img_tag_add_decoding_attr', '__return_false');

in theme functions.php, but it's not ideal.

I suspect this could be fixed by escaping the double quotes (for the lazy loading injection too), which shouldn't bother the HTML parser.

[rodricus]

@TimothyBlynJacobs Yes that's true. The javascript is loaded in a html section element after the HTML-code itself. The section is then loaded in the page if a user selects that block in the editor (the section is an ACF Gutenberg-block).

@joelmadigan Thank you for your information.

[mw108]

Same issue here as @joelmadigan - This new feature breaks NinjaForms when you have img tags in your fields.

The "magic" happens in wp-includes/media.php in function wp_filter_content_tags( ... ):

Forcing the decoding="async" attribute by using preg_match_all into all img tags is a bad solution. This has been discussed many times on StackOverflow: You should not parse HTML using regular expressions. Parse the DOM properly.

[TimothyBlynJacobs]

Moving to 6.1.1 for visibility.

[adamsilverstein]

I will take a look at this one to see how we can best avoid breaking image tags inside scripts or json. I would expect the lazy loading attribute to cause the same issue, is that not the case @rodricus?

[joelmadigan]

Replying to adamsilverstein:

I will take a look at this one to see how we can best avoid breaking image tags inside scripts or json. I would expect the lazy loading attribute to cause the same issue, is that not the case @rodricus?

They are implemented in the same way, so they absolutely can cause the same issue. The lazy loading part, however, has a limiting factor that it also checks for width & height attributes so it may not always affect the same images that the decoding one will.

[flixos90]

@adamsilverstein I wonder if we can find a reliable way to ignore img elements that appear in a script.

[adamsilverstein]

They are implemented in the same way, so they absolutely can cause the same issue. The lazy loading part, however, has a limiting factor that it also checks for width & height attributes so it may not always affect the same images that the decoding one will.

Ah, good point.

@adamsilverstein I wonder if we can find a reliable way to ignore img elements that appear in a script.

Yes, that was the direction I was thinking of heading. I'll give that a try to start.

[rodricus]

Replying to adamsilverstein:

I will take a look at this one to see how we can best avoid breaking image tags inside scripts or json. I would expect the lazy loading attribute to cause the same issue, is that not the case @rodricus?

Regarding the website I mentioned earlier, lazy loading attribute did not do any harm, only decoding="async".

[flixos90]

After further investigation with @adamsilverstein, we found what is probably the most straightforward and reliable fix here:

• The lazy-loading implementation is slightly different most importantly in the sense that it also checks for src=", which means it'll only touch img tags that use regular double quotes, i.e. no single quotes or escaped double quotes.
• This ensures it doesn't break any JS code where e.g. img tags may use other single quotes or escaped quotes.

So the best fix for this bug would probably be to add a similar check before adding any decoding attribute: If there is no src=" in the img tag, don't add the attribute. (If there is no src attribute, it's probably some special case that core shouldn't handle anyway.)

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

[adamsilverstein]

I'd like to chime in with an instance of the same issue in a slightly different context.

This new feature breaks NinjaForms when you have img tags in your fields.

Thanks @joelmadigan & @mw108 this helped me get to:

### Steps to reproduce ###

1. Install Ninja Forms
2. Create a form with single HTML field, inserting an image in the Default Value field
3. Insert the form on a page and view

So the best fix for this bug would probably be to add a similar check before adding any decoding attribute: If there is no src=" in the img tag, don't add the attribute. (If there is no src attribute, it's probably some special case that core shouldn't handle anyway.)

I did just this in https://core.trac.wordpress.org/ticket/56969

Also added some tests to cover the JSON case and the single quote case, both of which fail before the patch.

[adamsilverstein]

56969.diff​ adds the fix discussed above along with tests, mirroring changes in the PR.

[adamsilverstein]

@rodricus @mw108 @rodricus - can you please give the uploaded patch a test to verify it corrects the issue in your cases?

[flixos90]

I reviewed the PR, this looks good to go from my end. Just two quick nit-picks to address prior to commit.

[mw108]

Replying to adamsilverstein:

... - can you please give the uploaded patch a test to verify it corrects the issue in your cases?

Tested it and can confirm it works. Thanks. :)

Applied some of the more straightforward suggestions. @adamsilverstein would you be able to get this committed at some point today?

Yes, will do.

This seems Ok as temporary fix but as single quotes are a perfectly legitimate form of HTML, it would be good to follow up with something more elegant.

WordPress itself uses the quote types interchangeably and it seems to be fairly common in plugins too, especially when generating the markup from a set of variables.

​https://github.com/WordPress/wordpress-develop/blob/55d26534df75bbd13cc36a9e4e0881c5f8cc223c/src/wp-includes/pluggable.php#L2890

I agree it would be nice to have a more robust solution; this one seems ok for now - especially since the lazy attribute callback already has an identical check.

[adamsilverstein]

56969.2.diff​ includes all the latest changes/feedback from the PR.

Applied some of the more straightforward suggestions. @adamsilverstein would you be able to get this committed at some point today?

@desrosj I'll get this committed and backported first thing tomorrow if you don't beat me to it.

[peterwilsoncc]

In 54802:

Media: Prevent decoding attribute corrupting JSON data.

Workaround wp_img_tag_add_decoding_attr() potentially breaking JavaScript and JSON data by limiting the addition of the decoding attribute to image tags using unescaped double quoted attributes src attributes.

Props rodricus, TimothyBlynJacobs, joelmadigan, mw108, adamsilverstein, flixos90, desrosj, mukesh27.
Fixes #56969.

Merged in https://core.trac.wordpress.org/changeset/54802 / ​https://github.com/WordPress/wordpress-develop/commit/51032426b54113cf4acccd27de693ab7158a9f92

[peterwilsoncc]

Reopening for merging in to the 6.1 branch.

[desrosj]

In 54807:

Media: Prevent decoding attribute corrupting JSON data.

Workaround wp_img_tag_add_decoding_attr() potentially breaking JavaScript and JSON data by limiting the addition of the decoding attribute to image tags using unescaped double quoted attributes src attributes.

Props rodricus, TimothyBlynJacobs, joelmadigan, mw108, adamsilverstein, flixos90, desrosj, mukesh27, peterwilsoncc.
Merges [54802] to the 6.1 branch.
Fixes #56969.

[TobiasBg]

@peterwilsoncc: As it's polyfilled already in Core, should str_contains() be used instead of that strpos() check, "to eat our own dogfood"?

thanks @peterwilsoncc!

[peterwilsoncc]

@TobiasBg Good point, as WordPress 6.1.1 is currently in RC, I think it best to update this in WordPress 6.2 as part of the coding standards ticket, #56791.

Do you have the time to make up a quick patch/pull request and upload it to that ticket? If so, please mention me or @adamsilverstein when you upload it to bring it to my attention, this will help ensure you get props for the change.

[TobiasBg]

Done, see ticket:56791#comment:7. Thanks!