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

fix: ensure node is empty before stripping from content #113

Closed miguelpeixe closed 2 years ago

miguelpeixe commented 2 years ago

As pointed out by @tmtrademark in #111:

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.

This PR makes sure to check if the node has child nodes before checking for textContent length.

This issue does not occur with Newspack Campaigns active because the plugin transforms classic content into blocks before reaching SCAIP.

Closes #73 Closes #111

How to test

  1. Install and enable the Classic Editor plugin
  2. On the master branch make sure you don't have other plugins interfering with block content configuration (Newspack Campaigns)
  3. Add a new post with multiple paragraphs and a <p><img src="" /></p>
  4. Confirm the image does not render
  5. Check out this branch and confirm the image renders as expected
matticbot commented 2 years ago

:tada: This PR is included 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 PR is included in version 0.6.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: