s9_event_social: Set og:description from s9y_event_metadesc.

[th-h]

Currently s9y_event_social is using the first 200 chars of the entry body for the "og:description" meta tag.

The plugin s9y_event_metadesc makes it possible to specify a description for every entry that is used to set the "description" meta tag. This description, if present, should be used for the "og:description" meta tag, too. Using the first 200 chars can remain as a fallback solution.

At least the description from s9y_event_metadesc needs to be HTML-escaped. I think that goes for the first 200 chars from the body, too.

See http://board.s9y.org/viewtopic.php?f=10&t=20762&start=75 for a discussion (in German).

[yellowled]

Makes sense, but assigned to @onli to please have a look at it. Thanks, @th-h! :)

[th-h]

Perhaps someone has a better solution - I mostly pulled the code from s9y_event_metadesc, which seems to have another (better?) fallback. The full code there is:

$meta_description = $GLOBALS['entry'][0]['properties']['meta_description'];
if (empty($meta_description)) {
    $description_body = $GLOBALS['entry'][0]['body'];
    if (isset($GLOBALS['entry'][0]['plaintext_body'])) {
        $description_body = trim($GLOBALS['entry'][0]['plaintext_body']);
    }
    $meta_description = $this->extract_description($description_body);
}

The HTML escaping is done like this:

if (serendipity_db_bool($this->get_config('escape'))) {
    $md = (function_exists('serendipity_specialchars') ? serendipity_specialchars($meta_description) : htmlspecialchars($meta_description, ENT_COMPAT, LANG_CHARSET));
    [...]
} else {
    $md = $meta_description;
    [...]
}

I've got no idea what serendipity_db_bool($this->get_config('escape')) will be, so I just dropped that (as it didn't escape as necessary).

I'm not really happy with that kind of copy&paste code duplication anyway, but didn't see an easy way to avoid it. One could, I think, put the code in a central function for getting the|a description - but I'll leave that to the more experienced developers. :)

[onli]

I can't properly test this. The metadesc plugin just does not work for me, it is not setting any description, but it breaks how my theme is setting the entry title. It is supposed to just pick of text enclosed in the tags specified in the configuration, no?

Maybe me testing is not necessary. The code itself is smalland should work, at least if it works for you that is good enough for me.

My Feedback:

1. Don't use function_exists('serendipity_specialchars'), just use serendipity_specialchars directly. The plugin needs at least version 2.0, that function always exists.
2. However, I'm not sure you really want to escape the html here. Do you really want html tags in the description text? If no description is set via the metadesc-plugin, the social-plugin will run strip_tags() over the entry text before extracting the description. The metadesc-plugin should do the same, and we should just strip_tags here as well if it does not do so yet.

I'll merge the pull request, make the small change and augment the version number. But we can still discuss of course and do additional changes.

Thanks for the PR!

[th-h]

I can't properly test this. The metadesc plugin just does not work for me, it is not setting any description, but it breaks how my theme is setting the entry title. It is supposed to just pick of text enclosed in the tags specified in the configuration, no?

The plugin will add some input elements to the "advanced_options" part of the editor, see attached screenshot (German language).

Don't use function_exists('serendipity_specialchars'), just use serendipity_specialchars directly. The plugin needs at least version 2.0, that function always exists.

Okay, thanks! - I'm not familier with the s9y code base at all, so did not much more than copy & paste from the metadesc plugin (which seems to be quite a bit older). It got that check with commit /d2d298ab41905ff3c7ee7757c98c439efaf71f77 about 2 years ago (from you, it seems 😀 ).

However, I'm not sure you really want to escape the html here.

The user supplied description may contain single or double quotes which have to be escaped (or replaced by something else), I think, as they will break the meta element otherwise. A description like A little bit "over the top" ... will lead to <meta property="og:description" content="A little bit "over the top" ..." />.

Do you really want html tags in the description text?

Of course not! The metadesc plugin will apply strip_tags() later on, as it seems, but only if it discovers a <p> or </p>. Strange.

And I'm not sure what will happen to other markup, like markdown, in the first 200 chars ... The metadesc plugin does it wrong, while the social plugin does the right thing. I wonder why ...

I'll merge the pull request, make the small change and augment the version number. But we can still discuss of course and do additional changes.

Thanks! - We'll have to escape quotes, I think.

[onli]

The plugin will add some input elements to the "advanced_options" part of the editor, see attached screenshot (German language).

Thanks! That does not work for me. Well, separate bug, maybe something broke that in the current git master.

The user supplied description may contain single or double quotes which have to be escaped (or replaced by something else), I think, as they will break the meta element otherwise. … We'll have to escape quotes, I think.

You're perfectly right. Do you want to add that yourself in a second PR?

And I'm not sure what will happen to other markup, like markdown, in the first 200 chars ... The metadesc plugin does it wrong, while the social plugin does the right thing. I wonder why ...

The social plugin uses a very weird way to get the entry text. It could be that, or it is just that the markup plugin did not run yet.

[th-h]

The user supplied description may contain single or double quotes which have to be escaped (or replaced by something else), I think, as they will break the meta element otherwise. … We'll have to escape quotes, I think.

You're perfectly right. Do you want to add that yourself in a second PR?

Can do. - Wouldn't it work to just add serendipity_specialchars() again, or is there more to do?

And I'm not sure what will happen to other markup, like markdown, in the first 200 chars ... The metadesc plugin does it wrong, while the social plugin does the right thing. I wonder why ...

The social plugin uses a very weird way to get the entry text. It could be that, or it is just that the markup plugin did not run yet.

I have both plugins at the end of my plugin list, one after the other, so it's not the execution order. But I took that point to the forums, see Markup filtern - nicht nur HTML.

[onli]

Wouldn't it work to just add serendipity_specialchars() again, or is there more to do?

That would be enough :)

[th-h]

See #46, then.