Automattic / super-cool-ad-inserter-plugin

Insert ads, newsletter signup widgets, and calls to action in the middle of your posts!
https://wordpress.org/plugins/super-cool-ad-inserter/#description
32 stars 4 forks source link

Image tags inside of paragraph tags skipped by scaip_insert_shortcode using Classic Editor #111

Closed tmtrademark closed 2 years ago

tmtrademark commented 2 years ago

Reproduction Steps:

  1. Insert an image from Media Library into Classic Editor ( <img src="https://upload.wikimedia.org/wikipedia/commons/thumb/3/3a/Cat03.jpg/1200px-Cat03.jpg" />)
  2. Preview Post
  3. Image is missing

When the length of textContent is checked here https://github.com/Automattic/super-cool-ad-inserter-plugin/blob/master/inc/scaip-shortcode-inserter.php#L43, it doesn't account for the fact that <img> tags can and do get wrapped in <p> tags. So the enclosing <p> tags textContent is empty, making $text_length zero, so the node is skipped and not added back to $blocks.

I managed to work around this like so, which is not great because we end up running $dom->saveHtml on every node, regardless of whether we're going to include it. However, nodes that are actually empty do still get removed.

Maybe be related to some of the discourse on #73

            foreach ( $dom_body_elements as $index => $entry ) {
              error_log(print_r($entry, 1));
              // Trim whitespace, including non-breaking space.
              $block_html = $dom->saveHtml( $entry );
              $text_length = strlen( trim( $block_html, "\xC2\xA0\n" ) );
              if ( 0 === $text_length ) {
                continue;
              }

              $blocks[]   = [
                'blockName'    => 'core/html',
                'attrs'        => [],
                'innerBlocks'  => [],
                'innerHTML'    => $block_html,
                'innerContent' => [
                  $block_html,
                ],
              ];
            }
miguelpeixe commented 2 years ago

Hi @tmtrademark ! Thank you for catching that, I just pushed #113 with the fix :)

matticbot commented 2 years ago

:tada: This issue has been resolved in version 0.6.2-alpha.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

matticbot commented 2 years ago

:tada: This issue has been resolved in version 0.6.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: