Allow parent config to override empty values of more specific config

This is one of the difficulties of using Token - the logic checks to see if fields are empty before applying the token rather than after. This is something I need to look into.

Reroll attached.

I think an "override" is not strong enough. If a saved default field is emptied, one would expect it should no longer have any effect.

From what I can see, Metatag already does the following:
1) If a node's metatag field is not filled in, that tag is not saved, and the tag inherits from the parent defaults. I think this is already the case.
2) If a saved default field is not filled in, that tag is not saved, and the tag inherits from the parent default.

Thus, it should logically follow (but does not):
3) If a previously saved field is emptied, that tag should be dropped from the default record, and it should revert back to the above behavior.

To me, not following Rule 3 is a bug.

Seems it might sometimes be desirable to have empty values though and not fall back to the parent. In which case maybe the global fields should be left empty? or else the alternative is to have a setting for each metatag to specify whether or not the parent can override an empty value.

While it may be desirable to have defaults overriden by a NULL value, this should be the exception, not the rule. Thus, if there IS an override control, it should be to override inherited text with a NULL value. But my guess is it's not too common to want to empty or reduce your metatags. Usually people go overboard in the other direction.

I am submitting a patch that does the following two things:

1. Prevents Metadata from saving empty fields.
2. Prevents empty field values previously saved in the database from blocking parent configurations.

While this meets the functional requirements to prevent the unexpected behavior, it does not improve the UX, which can still confuse users. For that, I would propose THREE new things:

1. Adding an [empty] token (or something like it).
2. Filling the fields' 'default_value' with that configuration's saved values (imagine that), instead of the aggregated parent values.
3. Placing the aggregated parent values into the fields' 'placeholder' so that they display in gray and are replaced when clicking into the field.

These should remove all the ambiguity currently confusing users. I think I also see a number of related issues posted which may be likewise resolved.

Then we can talk about deprecating the shell game operation in Metadata where the aggregated parent values are deleted from each field just before they are saved.

Here's a problem - how is Metatag supposed to know if you are trying to reset a value to its default or you just don't want to output a specific meta tag?

I'm closing this in favor of #1798984: Improve meta tag form to clearly distinguish between overriding or reverting the default value (D9).

This would be a definite change in the current architecture so would require some sort of backwards compatibility to not skew existing sites.

Closed a duplicate: #2536104: Defaults and IF [token] empty then [token]

The tests need to be updated.

Doesn't apply against 1.13 or 1.x-dev

Rerolled the Patch

Thanks!
~Chirag

In https://www.drupal.org/project/metatag/issues/2850537#comment-12139609, jts86 (https://www.drupal.org/u/jts86) provided a simple patch that takes care of this for 8.x-1.x.

That patch no longer applies, but the code in it seems to do what is required here, i.e. allow a parent metatag to render if a token returns empty.

Since this works for some use cases, I rerolled it against 8.x-1.x and am posting it here.

After using the above for a bit, I found that it's problematic to attempt to do token replacements on a new entity without IDs as some entity-based tokens expect them and don't check for them. I've updated the patch with an isNew() check.

Emphasizing that this is not intended as a solution to the larger problem here, but it does potentially grant the ability to let parent tokens take over if child tokens return empty.

I tested the patch in #32 with the following:

• On admin/config/search/metatag/global set the "Description" basic tag to a string.
• On admin/config/search/metatag/node set the "Description" basic tag to [node:body]
• Added a node and included text in the body.
• Checked the metatags and saw the text from the body.
• Added a node and did _not_ include text in the body.
• Checked the metatags and the "Description" tag was not present.
• Applied the patch.
• Refreshed the second node and saw the "Description" tag now had the string from the global setting.
• Edited the node and added body text.
• Checked the metatags and saw "Description" now had the new body text.
• Edited again and removed the body text.
• Checked the metatags and saw it was back to the global string.

The patch applied but was offset a bit so I uploaded a new one with no other changes.

I applied the coder fix patch above but it added a . a the end of a commented out line of code that isn't even part of this patch so I removed that. This patch includes the other fix.

Seems this needs a bit of work, thanks for rerolling it Michelle.

There was an issue with last patches. It did not take into account the tags from the bundle.

Attach a patch and interdiff to fix this.

I'll try to take care of the tests soon.

Updating the patch in #39, which worked for me until I updated to Drupal 8.7.7 and Metatag 1.10.0

Thank you very much Kasey_MK!!!

Adding the failing test and the test with the fix patches.

Perhaps this is by design, but I just wanted to point out that the behavior in 1.10.0 (with patch #42 applied) has changed slightly from the previous expected results as described in comment #36. What I'm observing is easiest to explain with steps to reproduce:

• On admin/config/search/metatag/global set the "Description" basic tag to a string.
• On admin/config/search/metatag/node set the "Description" basic tag to [node:body]
• Add a node without a body value.
• Checked the metatags and "Description" is set to the default string value as expected.
• Edit the node and add a value for body.
• Checked the metatags and "Description" is still set to the default string. I would expect it to get set to the body value.
• Add a second node with a value for body.
• Checked the metatags and "Description" is set to the body value as expected.
• Edit the second node and remove body value.
• Checked the metatags and "Description" is null. I would expect it to revert to the default string.

After some debugging, I see that the new behavior is due to #3045460: MetatagFirehose::formElement() should pass current entity explicitly to metatag_get_default_tags(). If I remove the explicit entity value introduced in that change, I get the behavior I expect.

So it seems the new behavior is maybe by design, but -- at least for my use case -- not desired/expected.

Updated the patch to work with metatag_page_manager.

Other sub-modules may need updating, too.

Disabled empty values overriding on routes other than canonical (for e.g. on editing pages).
Overriding works only on the frontend.

FIxed the interdiff filename.

Rerolling patch #46

@AID_UA, what problem does your change fix? The description that you provide only describes the solution.

Patch from #48 works well.

In case anyone uses the patch at https://www.drupal.org/project/metatag/issues/2949963#comment-13929876, here's a re-roll of the patch here at #48 after the patch https://www.drupal.org/project/metatag/issues/2949963#comment-13929876a is applied.

Patch #48 rerolled.

I cant reroll the Patches for #51,0 also was rerolled for another issue.

Patch in #52 does not apply cleanly to the 1.30 release.

Edit: I'm dumb, of course that's meant for the 8.x+ version.

Here is a small PHP 8.1 fix. Basically strpos() no longer accepts NULL as the haystack (parameter 1)

Tested patch #55, works well.

Patch #55 does not seem to work with a headless approach. Metatags will be saved in the computed field. In a normal approach, where Drupal is rendering the output, the patch overwrites empty tags with global defaults:

// Merge with global defaults.
if (strpos($route_name, 'canonical') !== FALSE) {
  $metatags->overwriteTags(metatag_filter_empty_tokens($entity_metatags->get('tags'), $entity));
}

However when saving to the computed field, the route is not canonical and so global defaults are left untouched. When accessing JSON:API, only the computed field gets accessed and no merge can happen at that time.

I confirm that the patch doesn't work with the JSON:API module.

I updated the latest patch and added the fix for the computed field.