Problem/Motivation

The separator config appears to be applied too liberally? This can result in malformed metatags and schema metadata.

The separator config is managed at: /admin/config/search/metatag/settings

This issue appears to have been originally identified on Mar 26, 2021, but may have been overlooked.

Steps to reproduce

Case 1: Robots.txt > Invalid Markup
Set the separator config to a pipe (|).

Override contents of robots.txt in a node and observe the invalid rendering of the

<meta name="robots" content="index| follow| max-snippet:160| max-image-preview:large" />

Interestingly, it appears that the default checkboxes for these may also be lost when the separator is applied?

Case 2: Schema.org > Unexpected Array

Use the default separator config of a comma (,).

Schema (use Review and set the reviewBody to a field that includes text that includes commas (like a paragraph of text that includes commas). Then observe the reviewBody is now set to an array.

I suspect there may be other unintentional side-effects, but that's unclear.

Proposed resolution

TBD

Remaining tasks

Identify root cause. Evaluate potential places were use of separator config might be restricted or otherwise revisited.

User interface changes

TBD

API changes

TBD

Data model changes

TBD

Issue fork metatag-3578021

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

matt_paz created an issue. See original summary.

matt_paz’s picture

Issue summary: View changes
damienmckenna’s picture

Title: Unintended Consequence of Custom Separator? » Unintended consequence of custom separator?

Excluding Schema metadata, have you identified if this affects any included tags besides Robots?

matt_paz’s picture

I didn't pursue an exhaustive audit, but I checked a handful of other properties and didn't see any side-effects on those.

matt_paz’s picture

Issue summary: View changes
damienmckenna’s picture

Issue tags: +Needs tests

Let's update the test coverage to be more clear on this scenario.

jefftrnr made their first commit to this issue’s fork.

matt_paz’s picture

The one line change in MR:238 seems have corrected part of the issue. I suspect this issue was introduced in #3067803: Use custom delimiter instead of commas for multiple values and was just a minor oversight.

Additional test coverage seems desirable too, but that might be a larger lift.

Still need to continue working on rounding out the broader separator issue for metatags.

matt_paz’s picture

jefftrnr’s picture

@damienmckenna
My MR request now seems to be a mistake.
Earlier, I hadn't realized this patch for version 2.0.x-dev was not applied to version at 2.2.x-dev. https://www.drupal.org/project/metatag/issues/3367071

Our local version was still applying this patch. Once we removed the patch locally, my change broke the node/edit form again for the robots meta tag. The form worked correctly when my change was removed as well. Therefore, I think my MR from earlier on this page should be reverted. Sorry for the trouble.

Can you take care of that? Or should I apply a new MR request?