Plain text fields with tokens are rendered as markup leads to double-escaping characters

Uploaded patch for usage

The last submitted patch, 2: 3381449-2-tokenprocessor-doubleescaping.patch, failed testing. View results →

I like the solution this is putting in place. For next steps we'll just need to figure out why the test is failing, and determine if that test adequately covers the change or whether an additional one to test the new conditional branch will be needed.

The previous patch introduces use of the \Drupal\Core\Utility\Token::replacePlain() method whereas the existing test used the \Drupal\Core\Utility\Token::replace() method.

The attached patch updates the test to use the expected method.

I imagine patternkit may want to test plain and formatted text token replacements. Any recommendation on how you may want to see test coverage for both?

Also related to Patternkit's token replacement logic, I think it would be beneficial to use the replacement option ['clear' => TRUE], as documented here within Drupal core Token class, to remove the token if no replacement was found.

Should this be a separate issue or combined it with these changes and include test coverage for it here?

The work here looks good to me and I think it's almost there. I'm going to mark this back as "needs work" for the moment, however, pending the additional test case described below.

I imagine patternkit may want to test plain and formatted text token replacements. Any recommendation on how you may want to see test coverage for both?

Ya, we'll want to make sure and test both branches of the conditional that's getting added in there. I think the currently proposed change to target replacePlain() is fine for that test, but we'll want to test when replace() should be called instead, and implementing that as an additional test method on that same class will likely be the most straightforward solution.

Also related to Patternkit's token replacement logic, I think it would be beneficial to use the replacement option ['clear' => TRUE], as documented here within Drupal core Token class, to remove the token if no replacement was found.

I agree this would make a good addition. We would want to add a test case or assertions to validate that behavior as well, however. If we can get those added in at the same time, I'm fine with pushing it all through in this same issue. If not, It would probably make sense to split it out to a new issue.

Here's a new patch:

• Removes unnecessary comment with the apply method. Someone can read the token replace and replacePlain docblocks
• Add the ['clear' => TRUE] to the token processor class
• Updates the TokenProcessorTest class by removing unused code updating/adding replace and replacePlain test cases for the apply method.

I know the PatternFieldProcessorPluginManagerTest class is impacted by the ['clear' => TRUE] change. Let's see what other test fail as a result.

And another patch to update the PatternFieldProcessorPluginManagerTest test case. There were no other failing tests.

• add $optional_field_token to represent an non-required entity field that may not have a replacement value
• Set field_url as an empty string to represent the field without a value.
• Set the example pattern's field_url property to use $optional_field_token
• Add asserts to confirm the tokens are removed from output as expected with the ['clear' => TRUE] change to the PatternFieldProcessor TokenProcessor plugin.

+++ b/tests/src/Unit/Plugin/PatternFieldProcessor/TokenProcessorTest.php
@@ -54,127 +53,55 @@ class TokenProcessorTest extends UnitTestCase {
-  /**
-   * Tests applies method in various use cases.
-   *
-   * @covers ::applies
-   * @covers ::__construct
-   * @dataProvider providerApplies
-   */
-  public function testApplies(SchemaContract $propertySchema, bool $expected) {
-    $this->assertEquals($expected, $this->plugin->applies($propertySchema));
-  }
...
-  public function providerApplies() {

Why are we removing the test and provider for the applies() method?

Aside from this, the rest of the updates look good to me.

I think I naively removed them; this patch includes them.

slucero → credited minsharm → .

Merged for inclusion in the beta 8 release.

Retested the issue with this Patch File → and results looks good to me.

Steps to test

1) Go to any existing Drupal site with PK module already installed.
2) Go to content -> Add content. Select Basic Page as content type.
3) Give the title of the page to include a single quote character i.e. " ' " and click save.
4) Go to Layout Tab. (Layout Builder module should already be installed)
5) Click on Add block.
6) Add Patternkit example block.

Case 1 : Configure the pattern using the [node:title] token for the "Text" as well as "Formatted text" value, Content as Node from URL.

Results :

• Valid tokens with a provided context were replaced as expected

• 1) Observe the pattern is rendered replacing the [node:title] with the content's title.
• 2) Observe the single quote is rendered correctly for both the text and formatted text field.

Case 2 : Configure the pattern using the [node:title] token for the "Text" as well as "Formatted text" field, None as Node from URL.

Results :

An invalid token was removed. Observe pattern is rendered with blank "text" as well as "formatted text" field.
When a token is discovered, it is either replaced with the corresponding value or removed if no value can be identified. With the "Node from URL" selection set to "None", here we encounter the second scenario where the token has been identified, but no value is available for it and it is removed.

Automatically closed - issue fixed for 2 weeks with no activity.