Improve support for multi-value schema using Metatag's custom separator option

The workaround we used for an image CDN that used commas was to ensure the passed value was URL encoded on commas.

Is there a working workaround? I think it's a very big bug!

@smurfxx: Could you please test the patch, let us know if it resolves the problem for you?

I already tested the patch but, as @jjchinquist says, the function return always false even if the field is a multivalue.
In my specific case I have a text field with unlimited number of values because they are the recipe steps and it's impossible for me to avoid the use of commas in the text.
I wonder why use commas and not any other special chars (like pipes for example) to split the array, comma is always used in every language.

I see that the recipe metatag is also incomplete following this guidelines: https://developers.google.com/actions/content-actions/recipes but I'll open a dedicated issue for that.

I am also encountering this issue. I think the problem rather lies with not passing the $key value to processItem() when processing recipeInstructions.

  protected function neverExplode() {
    return [
      'streetAddress',
      'reviewBody',
      'recipeInstructions',
    ];
  }

reviewBody works because the key is correctly passed in. So the better solution would be to get recipeInstructions passing correctly passing in the key value and there's no need for the extra logic in the patch.

Here's a patch that does what I mentioned in the previous comment. It overrides output() for recipeInstructions metatag only.

Guess what, this happens with recipeIngredient too :(
The comma split is a really flaky way to handle these multivalue fields.

This is not a bug, it is the result of a decision, see #2933278: Add recipe schema. Basically there are two ways you might create steps, first as a text area that includes commas in the text to differentiate the steps, second as a collection of individual fields where each field represents a step. There is no way to know which is desired, but they need to be processed completely differently. The processing for either one will break the other. I chose to implement the first, as the most simple solution.

We cannot make this change without breaking any implementation that uses the current logic. That is the implementation that is in the wild, so that's important. I can't at the moment see any way to support both, but if someone can figure that out I'm open. What we can't do is just switch from one to the other.

Once #2990653: Add HowTo, FAQ, QA Page schemas (D8) is in, we will have a HowToStep all ready to go which could be used for recipes. It just has to be implemented in a way that any existing sites using text blocks still work.

The bigger problem here is that we have to explode on commas because the Token module uses a comma delimiter to join multiple value fields. This module relies on Metatag module to do the token replacements, and Metatag doesn't provide any way to override the delimiter. With a small patch, we could change this to a custom delimiter instead of a comma, which would then take care of most of the problems in this module.

I proposed a patch here: https://www.drupal.org/project/metatag/issues/3067803. If accepted, I can then implement an alter hook and use something else as a delimiter for multiple value fields (then explode on that instead) and quit breaking content that has commas.

Once I learned that I can create my own Tags in a custom module, this issue became easier to deal with, since it doesn't require a patch to get what I want.

Perhaps we can create a RecipeInstructionsV2 class that doesn't break the existing implementation? And provide it as optional or provide instructions in the README?

I'd love to take a stab at implementing this as an ItemList.

This won't pass tests because it relies on the Metatag patch, but if that patch gets in, the corresponding patch here would be this. With this code we can get rid of neverExplode(), which was a lame attempt to keep some of the worst offenders from breaking.

I tested the patch in #17 and it works good.

Thanks for the patch!

I have the same issue with QA Page.
When there is a comma, the acceptedAnswer is break in 2 rows

"acceptedAnswer": {
                    "@type": "Answer",
                    "text": [
                        "Comment éviter l’encrassement de vos persiennes ? Pour bénéficier d’une luminosité optimale",
                        "suivez notre guide d’entretien fait par des professionnels."
                    ],
                    "url": "xxx",
                },

Is there also a patch for this case ?

I have created a new patch for the same issue

Interdiff file for the same

This is a patch that allows you to define the separator from a configuration form.

This requires a patch to Metatag, https://www.drupal.org/files/issues/2020-08-07/metatag-n3067803-34.patch in #3067803: Use custom delimiter instead of commas for multiple values. With that applied, you need a few changes in this module. I am working on an update that is backwards compatible, it will work whether or not you have that patch applied, but you need that patch to configure a separator other than the comma.

Changing the description to better reflect what we are doing. This patch should work whether or not the Metatag patch is applied.

Ugh, typos.

Reroll patch from #34.

See also reroll of related metatag patch (issue #3067803: Use custom delimiter instead of commas for multiple values): https://www.drupal.org/project/metatag/issues/3067803#comment-13825255

@KarenS: One question about #15 where you explained:

The bigger problem here is that we have to explode on commas because the Token module uses a comma delimiter to join multiple value fields. This module relies on Metatag module to do the token replacements, and Metatag doesn't provide any way to override the delimiter. With a small patch, we could change this to a custom delimiter instead of a comma, which would then take care of most of the problems in this module.

Will a custom separator, different to comma, still work using token with multiple value fields? Example: In one place I need the custom separator for FAQ schema but everywhere else where multiple value token where used before I would expect that these continues to work (split by comma). Otherwise wouldn't it be better to define a custom separator for each metatag? ... Currently I don't have this usecase but it seems possible, doesn't it?

It seems that I found the answer to my questions from #36 in MetatagManager::generateRawElements():

        // Create options for handling token replacements, setting the current
        // language and a custom delimiter for multiple value fields in tokens.
        $options = [
          'langcode' => $langcode,
          'join' => $tag->getSeparator(),
        ];

This join option is then used in token.module function token_render_array(array $array, array $options = []) to join the token. So the custom separator should work well everywhere. Nice work KarenS.

Reroll patch from #35 for 8.x-2.x branch. I haven't tested it yet ... something is not working with the 8.x-2.x branch (I don't get the Pivot form for FAQ, but haven't had time to check what's wrong)

Regarding your comment on the Pivot for FAQ, might be #3182969: Wrong property_type for schema_qa_page_main_entity?

Postponed on #3067803: Use custom delimiter instead of commas for multiple values

The merge request is reliant on issue https://www.drupal.org/project/metatag/issues/3067803 being accepted and merged. But if that happens this merge request should fix it on the schema_metatag side.

P.s. this is my first time using the forking method of drupal.org I apologize if anything is done or configured wrongly.

The underlying functionality is going to be added in Metatag 8.x-1.20, and will be backported to D7 too.

It might be necessary to bump the Metatag requirement in composer.json when this is added.

I'm not a developer but this is the only place that I could find addressing the issue of using commas to separate items using this module.

I am trying to follow along. Is the proposed solution a setting in the module allowing the user to set a custom separator, such as a pipe, to separate items/values while still being able to use commas in strings of texts?

I ran into this issue creating recipes, where recipe instruction steps had commas. I'm now running into the same issue with FAQ pages.

There is very little in the documentation documenting pivoting or use of commas and I have not found any tutorials out in the wild. I am trying to train my clients, that aren't developers, on how to implement schema on pages and it has been very challenging.

Could not apply patch from #35 on current 8.x-2.x branch. I don't know exactly what changed but I rerolled it and it's working for me now. I removed hasSeparator() and getSeparator() methods from SchemaNameBase class to only use those same methods from the SchemaMetatagManager. I also changed the statically call of the getSeparator() method (which is not static) in SchemaNameBase by calling the metatag manager service in the constructor.

This can be unpostponed now that #3067803: Use custom delimiter instead of commas for multiple values was commited to at least the metatag 2.0.x branch.
The patch #2976935-46: Improve support for multi-value schema using Metatag's custom separator option worked for me.

This should be added to v3.

Rerolled.

Anyone want to work on test coverage for this?

@DamienMcKenna hello! I can work on some tests. I've done exactly 1 core test so be patient :)

@DamienMcKenna here's an updated patch that includes unit tests for explode().
Based on my understanding, that covers it, but let me know if I should include any more!

Very nice, thank you!

Hi team. What's the status on this issue? For me Patch from #53 is not working: "Could not apply patch! Skipping. The error was: Cannot apply pat ch https://www.drupal.org/files/issues/2023-01-27/schema_metatag-n2 976935-53.patch"

That means the patch needs to be rerolled.

How to reroll this patch to apply to 2.4? Thanks!

Patch #53 still applies to the 3.0.x branch, therefore it doesn't need to be rerolled. You might just need to update your site to use the new 3.0.x branch, until the changes are committed.

Re-created patch for 2.4

Making the latest 3.0.x patch visible.

Committed. Thank you everyone!

Don't know what I'm doing wrong:

Gathering patches for root package.
Removing package drupal/schema_metatag so that it can be re-installed and re-patched.
  - Removing drupal/schema_metatag (2.4.0)
Deleting web/modules/contrib/schema_metatag - deleted
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
Gathering patches for root package.
Gathering patches for dependencies. This might take a minute.
  - Installing drupal/schema_metatag (2.4.0): Extracting archive
  - Applying patches for drupal/schema_metatag
    https://www.drupal.org/files/issues/2023-05-25/schema_metatag_2976935_59.patch (Explode Separator)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2023-05-25/schema_metatag_2976935_59.patch

Generating autoload files
42 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
No security vulnerability advisories found

composer.json:

	"enable-patching": true,
	"patches": {
		"drupal/schema_metatag": {
			"Explode Separator": "https://www.drupal.org/files/issues/2023-05-25/schema_metatag_2976935_59.patch"
		}
	}

If you want this functionality please update to Schema Metatag 3.0.x-dev and Metatag 2.0.x-dev, or wait until I release the final versions next week.

(fixing the attributions)