comma separated list in og:image breaks if one component is empty

I think adding an array_filter() inside MetaNameBase::parseImageURL() might help fix this. Lets also add a test to make sure it does.

Here is an attempt at a patch (sorry no tests but a hopefully it's a good start). I started with the suggestion from #2, but things got more complicated!

• Need to strip unwanted commas even if there are no tags - in particular we may have "," in the case of "[token],[token]" where both tokens evaluated to nothing. In the end I deleted the optimisation strip_tags($value) != $value because I couldn't see a neat way to keep it, and I guess it won't make much difference. In fact strip_tags is not trivial so I may have speeded things up! The original code called strpos before checking for tags anyway so things were hardly optimal.
• Original code for "convert to absolute" only worked on the first URL. I've tried to fix that by moving it into the loop.
• Test for empty value should occur after parsing and trimming.
• Trimming should occur for inherited classes too (MetaPropertyBase, MetaItempropBase). Simplest fix I could see was to move trimming into parseImageURL.

Out of interest, does this solve the problem for you?

@DamienMcKenna Thanks for spending more time on this one.

Your patch in #4 doesn't solve the whole problem. Your patch is exactly what I expected from your comment in #2. In #3, I mentioned 4 extra problems that I found. I'll explain a little more.

The most important test case is if all of the components are empty, which your patch doesn't handle.

• The first problem is in parseImageURL because strip_tags($value) == $value so the array_filter is never called. My resolution (first bullet in #3) was to delete the line if (strip_tags($value) != $value) { because the contained code seems safe to run even if there are no tags. I think that this line is an attempted performance improvement that more often makes things slower - on many sites the tag will expand to valid images and so calling strip_tags is quite expensive and a waste of time.
• The second problem is in the output function (3 occurrences) that the test for empty value occurs before parsing and trimming, which is no good because the comma is there and so it's not yet empty. The check needs to be after once the comma has been removed (third bullet in #3).

Then I spotted two separate bugs that aren't actually a problem on my site, but I thought it might help others to fix them.

• Code fails to handle multiple images and all of them are relative (second bullet in #3).
• Trimming occurs for MetaNameBase but is missing for the other two (fourth bullet in #3).

If you like I can reroll my patch to exclude these two separate bug fixes.

Thanks for the extra explanation. Lets split off the separate bugs and I'll try to help with tests to cover this bug.

Thanks, here is the revised patch as requested in #6

I'm splitting off the output cleanup to #2911435: Streamline the HTML output logic.

The bare change.

@DamienMcKenna many thanks for your ongoing work. Is the status "needs review" still true? The new patch is only one line and looks sensible. I guess that the next step is covered by the tag "needs tests".

At this point I'm fine with the minor fix, the tests will be done in #2628472: Write tests for image handling.

Thanks for your work on this!

Twitter card image is still broken, even after this patch.
Og:image seems to work.

@aspilicious: Please open a new issue and describe the problem you're seeing with the twitter image tag. Thanks.

Just as a note, the twitter card image is broken as twitter doesn't like there to be a comma separated list of images.

See https://www.drupal.org/node/2628934 for handling multiple value metatags (like og:image) that would fix that.