Block API: Replace JSON-escaped quotation mark with unicode escape sequence

[aduth]

Fixes #6181

This pull request seeks to resolve an issue where contributors who submit a block containing an (escaped) quotation mark in the serialized attributes would have the resulting post content become malformed. The specific behavior results during post sanitization, which for users without unfiltered_html capability includes a number more filters. In particular, the wp_kses_stripslashes function causes escaped JSON quotes to become unescaped, thus resulting in an invalid parse in the next editor session.

Implementation notes:

It was proposed at #6181 (comment) to use entity-encoding on the quotation mark. However, this can result in a jarring end-user experience, where the encoded version would be displayed on next load:

Testing instructions:

Repeat steps to reproduce from #6181 (comment) , verifying that the unicode escape sequence is saved to post content and that the post restores itself correctly upon refresh.

[dmsnell]

Can we find a corresponding translation in the parser? I'm nervous about introducing asymmetries in the parser/printer system that could confuse people or introduce inconsistencies. For example, what happens if we want to write \" in a code block? Would it be preserved or transformed into \u0022?

Is there a way we can transform the quotation mark on save so that it never gets mangled by the WordPress backend?

[aduth]

I'm nervous about introducing asymmetries in the parser/printer system that could confuse people or introduce inconsistencies.

To be fair, is it being introduced? What about the other replacements here? I ask partly because I was hoping to find precedent in the parser 😄

Is there a way we can transform the quotation mark on save so that it never gets mangled by the WordPress backend?

I don't have the knowledge to speak to whether it's viable, but the documentation of the wp_kses_stripslashes function reads like a hacky fix ("It’s really weird, but the quoting from preg_replace(//e) seems to require this") that could potentially do for a better solution more accommodating of the slash'd quote.

[dmsnell]

To be fair, is it being introduced? What about the other replacements here? I ask partly because I was hoping to find precedent in the parser 😄

Huh. Well in my head there was precedence. Maybe it was primarily the HTML itself which was the other half of the equation.

If we can store that value in a code block and have it remain the same through the whole cycle then I think we're fine.

/me digs around to find those unserializers…

[aduth]

By default this wouldn't impact a code block, since it's only relevant for the JSON-serialized attributes. The code block sources its content from the markup. Manually updating the block to use comment attributes, the escaped form becomes:

<!-- wp:code {"content":"$foo = \u0022my \\\u0022escaped\\\u0022 string\u0022;"} -->
<pre class="wp-block-code"><code>$foo = "my \"escaped\" string";</code></pre>
<!-- /wp:code -->

Which apparently the parser converts back to its non-unicode form when restored:

{
  "blockName": "core/code",
  "attrs": {
    "content": "$foo = \"my \\\"escaped\\\" string\";"
  },
  "innerBlocks": [],
  "innerHTML": "\n<pre class=\"wp-block-code\"><code>$foo = \"my \\\"escaped\\\" string\";</code></pre>\n"
}

Further interesting to note is that the original problem (slash stripping) doesn't exist with the code block as implemented currently. I think it has to do with the behavior of wp_kses_split which only operates on text within HTML comments (i.e. serialized block attributes) or within the opening tags, not the content between the opening and closing tag.

[aduth]

Let me see if it'll be simple enough to write a unit test for this.

[aduth]

Added in cc34b0e.

[noisysocks]

Followed the testing steps locally—looks good! 👍

[aduth]

Thanks for reviewing @noisysocks ! As with #6620, I'll plan to land this shortly after the upcoming 3.0 release.

[gziolo]

Thanks for reviewing @noisysocks ! As with #6620, I'll plan to land this shortly after the upcoming 3.0 release.

I think I saw that one in the past. Not much time left before 3.1. Let's give it a spin before we start looking for a regression introduced after 3.0 :D