fix(perfmatters): match Jetpack social logos by identifier (NPPM-3167) - #995
Draft
wil-gerken wants to merge 2 commits into
Draft
fix(perfmatters): match Jetpack social logos by identifier (NPPM-3167)#995wil-gerken wants to merge 2 commits into
wil-gerken wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Perfmatters exclusions so Jetpack social icons render correctly across plugin and CDN asset paths.
Changes:
- Matches Jetpack social-logo styles by stable identifier.
- Adds coverage for plugin-hosted and CDN-hosted sharing styles.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
includes/plugins/class-perfmatters.php |
Broadens the social-logo exclusion. |
tests/unit-tests/perfmatters.php |
Tests actual substring-matching scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
Jetpack's social share icons can render as unstyled text until the reader interacts with the page. On mobile, they may not appear correctly until someone starts scrolling.
Newspack excludes Jetpack's social-icons stylesheet from Perfmatters' Remove Unused CSS using
_inc/social-logos. Jetpack 15.7 moved the file to_inc/build/social-logos/, so that exclusion no longer matches and Perfmatters delays the stylesheet.This changes the exclusion to
social-logos, which matches both the stylesheet URL and itssocial-logos-csshandle. It also continues to work when Jetpack's Asset CDN serves the file from a different path. Perfmatters matches against the<link>tag as first captured, before its own minifier can rewrite thehref, so the handle keeps matching even when the URL changes entirely.The change is a strict widening of the old exclusion, so the only new failure mode is a stylesheet being left unoptimised — never one being withheld.
Reference: NPPM-3167
How to test the changes in this Pull Request:
Perfmatters skips logged-in users, so test while logged out. Throughout, the
sharedaddystylesheet acts as a control: it matches a different exclusion entry that this PR does not touch, so it keeps itshrefin every state below. If a command returns nothing at all, the setup is wrong rather than the bug being absent.Reproduce on
release:On a site with Perfmatters active and Jetpack Sharing enabled, publish a post with share buttons.
Turn off Jetpack's Asset CDN with
wp jetpack module deactivate photon-cdn. It is on by default, and leaving it on can stop Perfmatters building the used-CSS bundle this half depends on.Run
wp perfmatters clear-used-cssandwp cache flush.Load the post two or three times while logged out, with a different query string each time (
?1,?2,?3), so page caching does not replay an earlier response.Run:
curl -s '<post-url>' | grep -oE "<link[^>]*(social-logos|sharedaddy)[^>]*>"Confirm the
social-logostag hasdata-pmdelayedstyleand nohref, and thatsharedaddyhas a realhref.The tag is the check, not the page. On a stock install Perfmatters builds its used-CSS bundle from this stylesheet and inlines it, so the icons can still paint while the link is withheld. Sites carrying other stylesheets usually get a bundle without the social-logos
@font-face, and there the icons stay unstyled until interaction.Verify this PR:
social-logostag now has a realhref, and thatsharedaddystill does.Also check the Asset CDN path:
This is the configuration the issue was reported under. It needs the page to load at least one stylesheet that Newspack's defaults do not already exclude — any third-party plugin with front-end CSS will do. A stock Newspack install has none, which is why the bundle never gets built there and this half silently passes without one.
wp jetpack module activate photon-cdn, and confirm the stylesheet is now served fromc0.wp.com(this requires a release-numbered Jetpack; alpha builds are not served from the CDN).release: run step 3, load the post logged out with?1and then?2, and run step 5's command. The first load builds the bundle; the second delays the stylesheet. Confirm thesocial-logostag hasdata-pmdelayedstylepointing at thec0.wp.comURL.c0.wp.comURL now has a realhref.If Perfmatters was activated, upgraded, or had its settings saved while this branch was checked out, the corrected entry is already in the database. Those paths read the options through Newspack's
option_perfmatters_optionsfilter and save the result back. Editing the PHP to_inc/social-logosthen does not restore the old behaviour, becauseset_defaults()re-merges the stored list. Rewriterucss_excluded_stylesheetsin theperfmatters_optionsoption as well. Front-end loads on their own never re-save it.Tests:
_inc/social-logosand runn test-php --group perfmatters. Two of the 15 tests fail, bothsocial-logoscases. The unit tests build the list from an empty array, so the stored option does not affect them.Other information:
Notes for reviewers
Test coverage: The new test uses Perfmatters' actual matching behavior — a case-insensitive substring match against the full
<link>tag — rather than just checking that the exclusion exists in the array. That matters here because_inc/social-logoswas still present even after it stopped matching Jetpack's stylesheet. Both the plugin-directory and Asset CDN forms are covered.Related exclusions: The two Jetpack entries below this one still use
plugins/jetpack/paths that do not match under the Asset CDN. That is a separate issue and will be handled in a follow-up.Existing Perfmatters settings: Sites that no longer receive our current Perfmatters defaults will not automatically get this corrected exclusion. That delivery problem is also being tracked separately.