Avoid JS error when pasing a caption without image

[youknowriad]

fixes #13527

I'm not sure this is the right fix but I noticed that sometimes the caption shortcode can be "image-less" which means body.firstElementChild is not defined causing a JS error. This patches the issue but I was wondering if we should just ignore the transformation entirely if the caption don't contain an image. (I'm not familiar enough with the shortcode transforms API to explore this).

[gziolo]

Looks legit but I would defer to others.

[ellatrix]

Test?

[ellatrix]

If there's no image, maybe we can create a placeholder with the caption? Unsure if that's what this PR does. I think in core the caption short code is also generalised to work with any media, so ideally it has multiple handlers depending on the contents, but this would be a big change. The core caption short code could be seen as a caption block that you can wrap around anything.

[jorgefilipecosta]

I managed to reproduce the bug by pasting:

[caption align="alignnone" width="198"] aaaa[/caption]

Not sure why in my setup pasting the long post linked on the issue did not cause the bug.

In this branch, no JS errors happen. But the result is not perfect we get an image placeholder where it is impossible to see the caption. When an image is set on the placeholder, the existing captions is overwritten, so the transformation we make is not very useful.
I'm not sure if isMatch transforms API works for shortcode but if it does, maybe we can add a check and ignore the transform in this case. Another option would be to just transform the caption text to a paragraph block but not sure what happens when multiple transforms exist for the same shortcode, (probably we would need an isMatch).
I feel the improvements I referred will probably evolve more explorations and I think what we have now is already a good improvement when compared to a JS error so I'm fine with merging this as soon as we have a test case exercising this condition.

[youknowriad]

Actually, the problem is probably different here because I checked again the original post, and I don't see any caption shortcode without image but when I paste this into Gutenberg

[caption id="attachment_2242" align="alignnone" width="800"]<img class="size-large wp-image-2242" src="https://florianbrinkmann.com/wp-content/uploads/2016/02/blog-uebersicht-800x633.png" alt="Abbildung 5: Screenshot von der Blog-Übersicht." width="800" height="633" /> Abbildung 5: Screenshot von der Blog-Übersicht.[/caption]

I noticed that shortcode.content contains the content but html encoded which means, it won't find the <img> tag. I'd personally prefer if one of you could dig deeper as it requires some knowledge about how pasting work.

[ellatrix]

@youknowriad For me this works:

I do have a REST API error with code rest_post_invalid_id...

[youknowriad]

Oh right! I'm dumb :P

I was actually copying from this link:

https://gist.githubusercontent.com/florianbrinkmann/4e9e4d23eaefb8b23484badddd848196/raw/2627e95996e362e5f177b8e5c58b8f38d95de56c/much-content.html

And in order for the browser to show the markup properly, it escapes it, which means if I copy it, I get the escaped version. I expect that the original author of the issue did the same thing.

So I guess the best fix here would be to ignore the caption shortcode entirely if there's not tag inside. Is this possible with our APIs?

[youknowriad]

For some reason, when trying to add a unit test here, it produces a paragraph instead of an image in the unit tests but when testing it produces an image with a caption and empty src (expected).

Help needed :) maybe we can just land it as is as it's an edge case.

[ellatrix]

I was actually copying from this link:

https://gist.githubusercontent.com/florianbrinkmann/4e9e4d23eaefb8b23484badddd848196/raw/2627e95996e362e5f177b8e5c58b8f38d95de56c/much-content.html

I'm not understanding fully. I see no caption short codes without an image in this content. What's the excepted content?

[youknowriad]

I'm not understanding fully. I see no caption short codes without an image in this content. What's the excepted content?

When you copy from that page on chrome, the content is escaped, which means the image becomes just a regular text. This also means this issue is unlikely to happen that often but I thought I'd just guard against it.

Another approach is to ignore the transform completely but there's no support for isMatch yet in the shortcode transform.

[ellatrix]

Ah, I get it now. I can reproduce. But in this case, all the content should be part of a preformatted block. Parsing the short code doesn't make any sense if the rest of the content is not being parsed.

At the moment we look for short codes before we look at the HTML, which is perhaps wrong. Your solution will fix the error, but the pasted result is still strange.

Not sure how to fix the short code problem immediately. Maybe we should look if it's part of escaped content and then leave it intact?

[youknowriad]

Related #10674

if we had isMatch working, the fix here would be to just ignore the shortcode transform.

[ellatrix]

Why would it solve the issue? I guess that the short code would be ignored, but others will still be transformed. So it solves a symptom but not the underlying issue. If someone pastes

<pre>[gallery ids="..."]</pre>

The shortcode transform should not happen.

[ellatrix]

Interestingly when I paste

<meta charset='utf-8'><pre style="color: rgb(0, 0, 0); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial; overflow-wrap: break-word; white-space: pre-wrap;">[caption id="attachment_2238" align="alignnone" width="718"]&lt;img class="size-full wp-image-2238" src="https://florianbrinkmann.com/wp-content/uploads/2016/02/screenshot-portfolio-uebersicht-mit-archivlink.png" alt="Abbildung 1: Übersicht der Arbeiten mit Archiv-Link auf der Website von Tomas van Houtryve Quelle: Screenshot von tomasvh.com am 1. November 2015" width="718" height="605" /&gt; Abbildung 1: Übersicht der Arbeiten mit Archiv-Link auf der Website von Tomas van Houtryve&lt;br /&gt;Quelle: Screenshot von tomasvh.com am 1. November 2015[/caption]</pre>

It does convert to a pre block.

[ellatrix]

Only if there is some text before it, the error occurs.

[youknowriad]

Why do you want to transform to a pre? It just means for me that it's not a shortcode that the current block handles.

[gziolo]

What's the status of this one? @youknowriad do you plan to continue working on it? There are so many comments here :)

[youknowriad]

We have two options:

• Implement this Transforms: "isMatch" does not work for shortcode transformation #10674 and use it here to avoid the transformation
• Merge this PR as is as a stop gap.

[gziolo]

Let's merge it and improve. WordPress 5.2 release date is coming up shortly :)

[youknowriad]

I justed noticed #14365 with the same fix but with tests, I'm fine deferring to that PR.

[gziolo]

Sounds good 👌

[gziolo]

I also reviewed #14365, it needs a small tweak but otherwise looks like it should land soon 👍