Bug 206327 – Add support for categories and custom post types to the social meta plugin

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Website

[Jon Davis]

Update the social meta plugin to support generic blog archive categories and custom post type archives.

[Jon Davis]

Created attachment 387874 [details]
Patch

[Devin Rousso]

Comment on attachment 387874 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387874&action=review

r=me

> Websites/webkit.org/wp-content/plugins/social-meta.php:35
> +        $description = get_bloginfo('name') . " blog archive.";

NIT: should we use "category" instead of `is_category()`?

> Websites/webkit.org/wp-content/plugins/social-meta.php:42
> +            $url = get_category_link($category_term_id);

Should we only override `$url` if `get_category_link(...)` returns a non-empty string (which indicates an error)?

> Websites/webkit.org/wp-content/plugins/social-meta.php:49
> +        $post_type_obj = get_post_type_object( $post_type );

Style: extra spacing around `(` and `)`

> Websites/webkit.org/wp-content/plugins/social-meta.php:50
> +

Style: unnecessary newline

> Websites/webkit.org/wp-content/plugins/social-meta.php:51
> +        if ( isset( $post_type_obj->description ) )

Ditto (49)

Also, should we null-check `$post_type_obj` too in case there was an error?

> Websites/webkit.org/wp-content/plugins/social-meta.php:54
> +        $url = get_post_type_archive_link($post_type);

Ditto (42)

[Jon Davis]

Created attachment 387935 [details]
Patch

[Devin Rousso]

Comment on attachment 387935 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387935&action=review

r=me, but this needs to be rebased

> Websites/webkit.org/wp-content/plugins/social-meta.php:32
> +        $description = "A repository of all ".get_bloginfo('name')."blog posts.";

Style: I think we should have spaces around `.`
Also, I think there should be a space before "blog".

> Websites/webkit.org/wp-content/plugins/social-meta.php:48
> +        $title = get_bloginfo('name')." Blog ".get_the_archive_title();
> +        $description = "A collection of ".get_bloginfo('name')." blog posts.";

Ditto (32)

> Websites/webkit.org/wp-content/plugins/social-meta.php:60
> +            preg_match_all('/<img.+src(set)?=[\'"]([^\s\'"]+).*[\'"].*>/i', $post->post_content, $matches);

This means that we will never use dark mode images.  Is that OK, or do we prefer that instead?

> Websites/webkit.org/wp-content/plugins/social-meta.php:61
> +            if (isset($matches[2][0])) $image_url = $matches[2][0];

Style: please put the body of the `if` on a separate line.

[Jon Davis]

Created attachment 387950 [details]
Patch

[Devin Rousso]

Comment on attachment 387950 [details]
Patch

r=me, thanks for iterating!

[WebKit Commit Bot]

Comment on attachment 387950 [details]
Patch

Clearing flags on attachment: 387950

Committed r254709: <https://trac.webkit.org/changeset/254709>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58658007>