craftcms / redactor

Edit rich text content in Craft CMS using Redactor by Imperavi.
https://plugins.craftcms.com/redactor
MIT License
100 stars 49 forks source link

Redactor fails to remove empty p tag on save #377

Closed mmikkel closed 2 years ago

mmikkel commented 2 years ago

Description

I'm seeing some odd/unwanted behaviour when authors attempt to clear/empty Redactor fields.

If an author clears out a Redactor field by first selecting all the text (i.e. Cmd+A) and then hitting the backspace button, the field value stored is always NULL (which is expected/wanted behaviour).

However, if a field contains multiple paragraphs, and the author clears out the field without first selecting all the text – for example, by putting the caret after the last character and then holding down backspace until the field is empty – the value stored will be an empty <p> tag with a single linebreak, i.e. this:

craft\redactor\FieldData#1
(
    [craft\redactor\FieldData:_pages] => null
    [craft\htmlfield\HtmlFieldData:_rawContent] => '<p><br /></p>

'
    [Twig\Markup:content] => '<p><br /></p>

'
    [Twig\Markup:charset] => 'UTF-8'
)

Visually, the Redactor field appears empty after saving, and if the entry is re-saved again (without touching the field) the empty <p> appears to be removed. But, this isn't something that is easy for authors to catch, and results in a lot of {% if entry.text %} template conditionals evaluating to true, even if there isn't actually any content in the field.

Steps to reproduce

  1. Add multiple paragraphs of text to a Redactor field in an entry
  2. Save the entry
  3. Remove all the text in the Redactor field by placing the caret after the last character in the field, then holding down the backspace button
  4. Re-save the entry
  5. Confirm that the field value saved is an empty <p> tag with a single <br/> inside.

Additional info

andris-sevcenko commented 2 years ago

@mmikkel Is it saved with a <br> or <br/> inside?

mmikkel commented 2 years ago

Here's the relevant part of the payload for the actions/entry-revisions/save-draft request that triggers when the field is emptied – looks like it's saved as <p><br></p>:

CleanShot 2022-03-31 at 15 49 04

mmikkel commented 2 years ago

Here's a quick screencast, if it helps:

https://user-images.githubusercontent.com/298510/161071315-86f1f491-f9ee-457f-a1a1-00c54fc2b8b7.mp4

andris-sevcenko commented 2 years ago

I understand the issue, I just don't get why it's not being cleared up.

Any chance you could add some debugging code around these parts? It should be taking care of this so I'm just a little perplexed.

mmikkel commented 2 years ago

I think it's due to some trailing whitespace in the value submitted from the Redactor field, so the conditional at https://github.com/craftcms/html-field/blob/fd3d400643fccbe27583102f581640f465987a06/src/HtmlField.php#L94 evaluates to false.

If I add a simple trim() to the $value, it works as expected, i.e.

if (in_array(\trim($value), ['<p><br></p>', '<p>&nbsp;</p>'], true)) {
   return null;
}
brandonkelly commented 2 years ago

Fixed via craftcms/html-field v1.0.7 and 2.0.2.