Bug 206327

Summary: Add support for categories and custom post types to the social meta plugin
Product: WebKit Reporter: Jon Davis <jond>
Component: WebKit WebsiteAssignee: Jon Davis <jond>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, jond, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Jon Davis 2020-01-15 17:05:05 PST
Update the social meta plugin to support generic blog archive categories and custom post type archives.
Comment 1 Jon Davis 2020-01-15 17:13:12 PST
Created attachment 387874 [details]
Patch
Comment 2 Devin Rousso 2020-01-15 17:26:38 PST
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)
Comment 3 Jon Davis 2020-01-16 11:19:47 PST
Created attachment 387935 [details]
Patch
Comment 4 Devin Rousso 2020-01-16 11:43:17 PST
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.
Comment 5 Jon Davis 2020-01-16 13:28:38 PST
Created attachment 387950 [details]
Patch
Comment 6 Devin Rousso 2020-01-16 13:44:27 PST
Comment on attachment 387950 [details]
Patch

r=me, thanks for iterating!
Comment 7 WebKit Commit Bot 2020-01-16 14:26:50 PST
Comment on attachment 387950 [details]
Patch

Clearing flags on attachment: 387950

Committed r254709: <https://trac.webkit.org/changeset/254709>
Comment 8 WebKit Commit Bot 2020-01-16 14:26:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2020-01-16 14:27:15 PST
<rdar://problem/58658007>