Problem/Motivation

In our site improper empty og:image tags were generated. It was, because we used a definition for the og:image field where multiple values were allowed. If all of the fields have value, everything is ok. The MetatagManager.php replaces the given tokens and creates a proper temporary value which will be processed well. But if any of them is missing, the result will not be proper, the html source will also contain empty "og:image" meta tags.

Steps to reproduce

Set the following metatag to Open Graph, image:

[node:field_image:max_1300x1300], [node:field_images:max_1300x1300], [node:field_logo:max_1300x1300], [node:field_result_logo:max_1300x1300]

It will generate 3 og:image metas to the header of the content pages (unless you have such fields with content in them) with the base url of the site.

Proposed resolution

I have a kind of solution patching the MetatagManager.php where the empty values are removed. However some might create a more elaborate solution.

This bug has been already reported by me here: https://www.drupal.org/project/metatag/issues/3182954

The status of that ticket is "Closed (won't fix)" + I was requested to create a new issue if the modifications in #3067803 won't fix this one. So here it is, I think #3067803 did not addressed this bug or has not been included in the code for more than a year.

CommentFileSizeAuthor
#5 3378434-5.patch628 bytesabhinand gokhala k
#4 3378434-4.patch630 bytesheykarthikwithu
metatag_module_bugfix_patch_has_been_added_for_v_2_0_0.patch1014 byteszolt_toth

Issue fork metatag-3378434

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:

About issue forks

Comments

zolt_toth created an issue. See original summary.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Improper handling of multiple valued, comma separated tag configs (2.0.*) » Empty values not removed from multi-value meta tags
Version: 2.0.0 » 2.0.x-dev
Status: Active » Needs review

Thank you for reporting this bug, and I see that the work on multi-value tag output didn't resolve it.

Let's see what the testbot says about the patch.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

The patch uses the wrong paths, so it needs to be corrected.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
commented
StatusFileSize
new 3378434-4.patch630 bytes

Only rewriting the patch with correct patch (should see will it on work on multi-value tag output)

abhinand gokhala k’s picture

abhinand gokhala k commented
StatusFileSize
new 3378434-5.patch628 bytes
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 3378434-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

xamount’s picture

xamount
Primary language English
commented

The patch at #5 fixes the issue for me. Thanks!

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

pradhumanjain2311 opened merge request !111

xamount’s picture

xamount
Primary language English
commented
Status: Needs work » Needs review
naveenvalecha’s picture

naveenvalecha
He/Him
commented
Related issues: +#3491230: Custom Meta tags: Handle empty values

Added related issue

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

The change needs to accommodate "empty" values that are not blank, i.e. the number zero.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 2.0.x-dev » 2.1.x-dev
naveenvalecha’s picture

naveenvalecha
He/Him
commented

@damienmckenna How to reproduce with values number 0. Could you help with the steps to reproduce?
The above steps to reproduce steps are not generating values number 0

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

eleonel’s picture

eleonel
he/him
Primary language Spanish
Location Itoshima 🇯🇵
commented
Status: Needs work » Needs review

The MR now strips empty/whitespace-only values from multi-value meta tags while keeping a literal 0 (per #13). Fix is in MetaNameBase::output(), gated on multiple(), so single-value tags are untouched.

Reverted the old array_filter() in MetatagManager (it dropped 0 and hard-coded the comma).

Added Kernel tests for empty removal, the zero case, and a custom separator. Also included two small pre-existing D11 test/CSS fixes so the pipeline runs.

radimklaska’s picture

radimklaska
Primary language Czech
Location 🇨🇿Czech Republic
commented

Tested MR !111 locally on a Drupal 11.3.10 site:

* empty values removed from multi-value tags, literal "0" preserved
* no regressions on the site

Works as expected.

The "merge error" on the MR is just the target branch: it still targets 2.0.x while 2.1.x was merged into it. Verified locally, it merges cleanly into 2.1.x.

skipper-vp’s picture

skipper-vp commented

Reviewed the MR locally against 2.1.x.

The GitLab diff looks large because the MR is still targeting 2.0.x, but when compared locally against 2.1.x the actual diff is limited to:

- metatag_extended_perms/tests/src/Functional/PermissionsTest.php
- src/Plugin/metatag/Tag/MetaNameBase.php
- tests/src/Kernel/MetatagManagerTest.php

The fix in MetaNameBase::output() looks correct to me. It skips empty/whitespace-only values only for multi-value tags, and uses a strict empty-string check so the literal value "0" is preserved.

The added kernel coverage also looks appropriate. It covers empty values, whitespace-only values, "0", custom separators, and a single-value tag containing the separator character.

Moving to RTBC.

skipper-vp’s picture

skipper-vp commented
Status: Needs review » Reviewed & tested by the community
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 2.1.x-dev » 2.2.x-dev
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3548158: Plan for Metatag 2.2.1

Let's get this into the next release.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Fixed

Committed. Thank you!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.