Skip to content

some cleanups and a simpler image handling - #46

Merged
pfefferle merged 12 commits into
mainfrom
simplify-opengraph
Sep 18, 2026
Merged

pfefferle merged 12 commits into
mainfrom
simplify-opengraph

Conversation

@pfefferle

@pfefferle pfefferle commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner

I went through the whole plugin and cleaned up a lot of duplicated code. The biggest change is the image handling.

Images: Instead of four filter callbacks that each scan the post from a different angle, there is now one default callback that walks a list of sources (thumbnail, content, attached images) and stops as soon as the list is full. The content source does one pass over the raw HTML and reads the wp-image-{id} class, the same way core does. That also catches gallery, media & text and nested images, which the old block parser missed. Only images without the class go through the (slow) URL lookup.

Twitter card: The "more than one image" rule was a leftover from the old gallery card type that Twitter removed in 2015. The card is now summary_large_image whenever a singular post has an og:image, summary otherwise. The property filters get the metadata collected so far as a second argument, so the Twitter card does not run the whole image chain a second time.

Tests: phpunit setup based on the ActivityPub plugin, with tests for all image variants (blocks, classic content, URL fallback, limits, fallback images) and the rest of the metadata. One of the tests found an old bug: merging into an existing prefix attribute dropped the space.

Minimum WordPress version is now 6.2 (the readme still said 2.3, which was not true for years). That is the version with WP_HTML_Tag_Processor, so the class check and the str_starts_with polyfill are gone. PHP stays at 7.4.

Some public functions are gone: opengraph_block_image, opengraph_parsed_image, opengraph_attached_image, opengraph_ensure_max_image and opengraph_site_supports_blocks (and its filter). The new opengraph_image_sources filter is the way to add or remove a source now. I think this should be a minor version bump, not a patch release.

Two things I am not sure about: posts without own images now get a large Twitter card with the site icon, and the og:image list is no longer capped for images that third party filters add.

This also supersedes #39: the fatal error with The Events Calendar came from the get_the_content() call in the old block collector, which is gone. There is a test for it now.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved PHP 7.4 compatibility, image collection, test setup/CI, and release metadata issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request refactors Open Graph image handling, raises the minimum WordPress version to 6.2, and adds PHPUnit tooling and CI coverage.

Changes:

  • Consolidates image discovery and updates Twitter card behavior.
  • Replaces legacy image helpers with the new image-source filter.
  • Adds tests, local tooling, CI configuration, and compatibility metadata.
File summaries
File Summary
tests/phpunit/tests/class-test-opengraph.php Metadata regression tests
tests/phpunit/tests/class-test-opengraph-images.php Image discovery and limit tests
tests/phpunit/includes/class-opengraph-testcase.php Shared PHPUnit helpers
tests/phpunit/bootstrap.php PHPUnit bootstrap
readme.md WordPress requirement metadata
phpunit.xml.dist PHPUnit configuration
phpcs.xml Coding-standard configuration
package.json Local wp-env scripts
opengraph.php Core metadata and image-handling refactor
composer.json PHPUnit dependencies and test scripts
bin/install-wp-tests.sh WordPress test-suite installer
.wp-env.json Local test environment configuration
.gitignore Development artifact exclusions
.github/workflows/phpunit.yml PHPUnit CI workflow
.github/workflows/phpcs.yml PHPCS CI workflow
.gitattributes Distribution archive exclusions
Review details

Suppressed comments (5)

.github/workflows/phpunit.yml:15

  • The test setup now depends on bin/install-wp-tests.sh, but neither path list triggers this workflow when that script changes. A later fix to the installer could therefore merge without running the tests; include the installer in both the push and pull-request path filters.
      - '**/*.php'

composer.json:33

  • The new composer test command installs the test suite with database host test-db and SKIP_DB_CREATE=true, but no test-db service or pre-created database is defined anywhere in this repository; the wp-env configuration also does not provide that host. A clean local checkout therefore cannot run composer test as advertised—either provision that service/database or use the same connection/setup as the CI command.
            "bin/install-wp-tests.sh opengraph-test root opengraph-test test-db latest true",

opengraph.php:759

  • This second str_starts_with() call is also unavailable on the declared PHP 7.4 floor. Sites that enable OPENGRAPH_STRICT_MODE will hit an undefined-function fatal while rendering every metadata request; replace these checks with a PHP 7.4-compatible implementation.
		} elseif ( str_starts_with( $key, 'twitter:' ) || str_starts_with( $key, 'fediverse:' ) ) {

opengraph.php:397

  • This matches the substring anywhere in the class attribute, so a class such as not-wp-image-123 is treated as attachment 123 even though it is not a wp-image-{id} class. Use class-token boundaries so unrelated class names cannot select the wrong attachment.
	if ( is_string( $class ) && preg_match( '/wp-image-([0-9]+)/i', $class, $matches ) ) {

opengraph.php:371

  • get_post_field() defaults to the display context, so this applies the_content filters instead of scanning the raw stored markup. That can reintroduce content-filter side effects (and undermines the fix for the #39 failure); request the raw context explicitly here.
	$tags = new WP_HTML_Tag_Processor( get_post_field( 'post_content', $post_id ) );
  • Files reviewed: 14/17 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread opengraph.php
Comment thread opengraph.php
Comment thread opengraph.php Outdated
Comment thread opengraph.php
Comment thread readme.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical CI and password-protection issues remain, along with metadata and release-version concerns.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

opengraph.php:822

  • $metadata is the current metadata, and this callback is an extension point. The previous implementation appended term names to article:tag, preserving values supplied by earlier opengraph_metadata callbacks; replacing the key here now silently drops those custom tags. Merge the plucked terms into the existing value (with an empty-array fallback) instead.

.github/workflows/phpunit.yml:68

  • This repeats the invalid dot access to the hyphenated php-versions matrix key, so the environment passed to PHPUnit is invalid. Use bracket notation or rename the matrix key.
          PHP_VERSION: ${{ matrix.php-versions }}

opengraph.php:169

  • Removing the final max-images filter means a normal opengraph_image callback that runs after the priority-5 default (for example, at priority 10) can append more than opengraph_max_images(). The default callback only caps the list at its own execution point, so the documented limit is no longer enforced; retain a final cap after all image filters or enforce it when metadata is finalized.
	add_filter( 'opengraph_image', 'opengraph_fallback_image', 35 );

opengraph.php:12

  • The new compatibility metadata leaves the plugin Version and readme Stable tag at 2.0.2, even though the PR removes public functions, raises the minimum WordPress version, and describes a minor version bump. A release built from this change would still identify itself as the old 2.0.2 release; bump the plugin header and readme version together before publishing.
 * Requires at least: 6.2
 * Requires PHP: 7.4

readme.md:5

  • This PR raises the minimum WordPress version and removes public functions, but the plugin header and readme stable tag still identify the release as 2.0.2. That would ship the breaking change under the old patch version and does not match the requested minor bump; update the release version consistently in the header, stable tag, and changelog.
- Requires at least: 6.2
  • Files reviewed: 14/17 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread .github/workflows/phpunit.yml
Comment thread .github/workflows/phpunit.yml
Comment thread opengraph.php Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unresolved moderate findings affect image limits, PHP 7.4 compatibility, and release version metadata.

Review details

Suppressed comments (4)

opengraph.php:169

  • The final max-image callback was removed, but opengraph_default_image() only slices the values present at priority 5. A plugin that appends images at a later priority (for example 10) can now make og:image exceed OPENGRAPH_MAX_IMAGES; opengraph_fallback_image() returns early and does not cap it. Keep a final cap callback after third-party filters and add a regression test for a later-priority filter.
	add_filter( 'opengraph_image', 'opengraph_fallback_image', 35 );

opengraph.php:406

  • PHP 7.4 is still supported, but the removed polyfill leaves str_starts_with() undefined on this URL-fallback path. The new test_content_image_by_url() exercises this line, so the PHP 7.4 CI job will fatal; use a PHP 7.4-compatible prefix check here (or restore the polyfill), and likewise fix the strict-mode call below.
	if ( ! is_string( $src ) || ! str_starts_with( $src, wp_get_upload_dir()['baseurl'] ) ) {

opengraph.php:11

  • The plugin header and readme.md still identify this as version 2.0.2, although this change removes public functions, adds a replacement filter, and raises the minimum WordPress version; the PR description calls for a minor bump. Update the plugin version and Stable tag so this release is distinguishable from 2.0.2.
 * Requires at least: 6.2

readme.md:5

  • The PR description calls for a minor version bump, but the plugin header and the readme Stable tag still identify this code as 2.0.2. Shipping the removed public callbacks and new minimum WordPress requirement under the existing release metadata would leave WordPress treating it as the already-published 2.0.2 release; please bump the header, Stable tag, and changelog together, or clarify that versioning is intentionally deferred.
- Requires at least: 6.2
  • Files reviewed: 14/17 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@pfefferle
pfefferle merged commit f16b9a0 into main Sep 18, 2026
5 checks passed
This was referenced Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants