| @@ -25,8 +25,19 @@ | ||
| 25 | 25 | * |
| 26 | 26 | * Adding a rule: drop another `if (…) $out[] = …` block in run(). |
| 27 | 27 | * Rules are independent — keep them small + concrete + factual. |
| 28 | 28 | * |
| 29 | + * An add-on adds one from outside via `xspeed_pro_audit_suggestions` | |
| 30 | + * instead — see contributed() for the shape it has to return and why | |
| 31 | + * that path is not gated by already_active(). | |
| 32 | + * | |
| 33 | + * If the suggestion maps to a single Pro module, add it to | |
| 34 | + * BACKING_MODULE and guard the rule with `! self::already_active($id)`. | |
| 35 | + * A rule that fires on site state alone keeps nagging a customer who | |
| 36 | + * already bought and enabled the feature — and any consumer rendering | |
| 37 | + * the audit as a call-to-action (the Hub's "Enable" button) then shows | |
| 38 | + * a control that can never clear. (#187) | |
| 39 | + * | |
| 29 | 40 | * @package XSpeed |
| 30 | 41 | */ |
| 31 | 42 | |
| 32 | 43 | declare(strict_types=1); |
| @@ -41,8 +52,21 @@ | ||
| 41 | 52 | public const SEVERITY_MED = 'med'; |
| 42 | 53 | public const SEVERITY_LOW = 'low'; |
| 43 | 54 | |
| 44 | 55 | /** |
| 56 | + * Bounds on what `xspeed_pro_audit_suggestions` may add. | |
| 57 | + * | |
| 58 | + * The audit is rendered in a dashboard card and returned verbatim by the | |
| 59 | + * MCP `get_pro_audit` tool, so an add-on that appends 500 rows or a | |
| 60 | + * paragraph of prose degrades both. The caps are generous against any | |
| 61 | + * honest use — no native rule comes close — and exist so a buggy | |
| 62 | + * contributor can't make the payload the problem. | |
| 63 | + */ | |
| 64 | + private const MAX_CONTRIBUTED = 10; | |
| 65 | + private const MAX_REASON_LEN = 400; | |
| 66 | + private const MAX_FACT_LEN = 32; | |
| 67 | + | |
| 68 | + /** | |
| 45 | 69 | * Snapshot of state every rule needs. Computed once per audit run |
| 46 | 70 | * so we don't read the same option 8 times. |
| 47 | 71 | * |
| 48 | 72 | * @param array|null $totals_override Test injection — Brain Monkey |
| @@ -61,10 +85,11 @@ | ||
| 61 | 85 | $totals = $totals_override; |
| 62 | 86 | } elseif ( class_exists( '\\XSpeed\\Hit_Counter' ) ) { |
| 63 | 87 | $totals = Hit_Counter::totals_24h(); |
| 64 | 88 | } else { |
| 65 | - $totals = array( 'hits' => 0, 'misses' => 0, 'ratio' => 0.0 ); | |
| 89 | + $totals = array( 'hits' => 0, 'misses' => 0, 'excluded' => 0, 'ratio' => 0.0 ); | |
| 66 | 90 | } |
| 91 | + $cloudflare = $opts( 'cloudflare' ); | |
| 67 | 92 | return array( |
| 68 | 93 | 'cache' => $opts( 'cache' ), |
| 69 | 94 | 'minify' => $opts( 'minify' ), |
| 70 | 95 | 'lazy' => $opts( 'lazy' ), |
| @@ -69,9 +94,9 @@ | ||
| 69 | 94 | 'minify' => $opts( 'minify' ), |
| 70 | 95 | 'lazy' => $opts( 'lazy' ), |
| 71 | 96 | 'gzip' => $opts( 'gzip' ), |
| 72 | 97 | 'browser_cache' => $opts( 'browser-cache' ), |
| 73 | - 'cloudflare' => $opts( 'cloudflare' ), | |
| 98 | + 'cloudflare' => $cloudflare, | |
| 74 | 99 | 'cdn' => $opts( 'cdn' ), |
| 75 | 100 | 'database' => $opts( 'database' ), |
| 76 | 101 | 'preloader' => $opts( 'preloader' ), |
| 77 | 102 | 'heartbeat' => $opts( 'heartbeat' ), |
| @@ -77,18 +102,408 @@ | ||
| 77 | 102 | 'heartbeat' => $opts( 'heartbeat' ), |
| 78 | 103 | 'cache_enabled' => class_exists( '\\XSpeed\\Settings' ) |
| 79 | 104 | ? ! empty( Settings::get()['cache_enabled'] ) |
| 80 | 105 | : false, |
| 106 | + // An edge cache (Cloudflare) in front means the origin hit ratio is | |
| 107 | + // only the origin layer — hits served at the edge never reach PHP — | |
| 108 | + // so a low number is an attribution artefact, not a cache problem. | |
| 109 | + // Rule 2 must not fire an upsell off it. (#118) | |
| 110 | + 'edge_cache' => ! empty( $cloudflare['enabled'] ), | |
| 81 | 111 | 'totals_24h' => array( |
| 82 | - 'hits' => (int) ( $totals['hits'] ?? 0 ), | |
| 83 | - 'misses' => (int) ( $totals['misses'] ?? 0 ), | |
| 84 | - 'total' => (int) ( $totals['hits'] ?? 0 ) + (int) ( $totals['misses'] ?? 0 ), | |
| 85 | - 'ratio' => (float) ( $totals['ratio'] ?? 0.0 ), | |
| 112 | + 'hits' => (int) ( $totals['hits'] ?? 0 ), | |
| 113 | + 'misses' => (int) ( $totals['misses'] ?? 0 ), | |
| 114 | + // 404s + bots, kept out of the ratio denominator. (#118) | |
| 115 | + 'excluded' => (int) ( $totals['excluded'] ?? 0 ), | |
| 116 | + 'total' => (int) ( $totals['hits'] ?? 0 ) + (int) ( $totals['misses'] ?? 0 ), | |
| 117 | + 'ratio' => (float) ( $totals['ratio'] ?? 0.0 ), | |
| 86 | 118 | ), |
| 87 | 119 | ); |
| 88 | 120 | } |
| 89 | 121 | |
| 90 | 122 | /** |
| 123 | + * Which Pro module backs each suggestion id. A suggestion whose module | |
| 124 | + * is installed AND switched on is not an upsell any more — the site | |
| 125 | + * already has the thing we'd be selling. (#187) | |
| 126 | + * | |
| 127 | + * Ids without an entry (analytics fallback, white-label, …) are always | |
| 128 | + * eligible; absence here means "no single module answers this". | |
| 129 | + */ | |
| 130 | + private const BACKING_MODULE = array( | |
| 131 | + 'webp-avif' => 'images', | |
| 132 | + 'rum' => 'rum', | |
| 133 | + 'critical-css' => 'critical-css', | |
| 134 | + ); | |
| 135 | + | |
| 136 | + /* | |
| 137 | + * `cloudflare-apo` is deliberately NOT here. | |
| 138 | + * | |
| 139 | + * APO's on/off state lives at Cloudflare — `Cloudflare_Apo::status()` | |
| 140 | + * is a live GET against their API, and our own options carry only the | |
| 141 | + * values we PUSH to it (cache_level, browser_ttl). Nothing local | |
| 142 | + * records whether it is on. The audit runs on every dashboard load and | |
| 143 | + * on Free installs with no credentials, so it must not make a network | |
| 144 | + * call to find out. | |
| 145 | + * | |
| 146 | + * The first cut mapped it to an `enabled` key that the module never | |
| 147 | + * writes, which read as "always eligible" — the right OUTCOME by | |
| 148 | + * accident, via a check that could never fire. Stating the limitation | |
| 149 | + * is better than a guard that looks like it works. If a cached | |
| 150 | + * APO-state option is added later, give it a probe below and restore | |
| 151 | + * the mapping. (#187 review) | |
| 152 | + */ | |
| 153 | + | |
| 154 | + /** | |
| 155 | + * Ids whose module records its on/off state somewhere other than a | |
| 156 | + * plain `enabled` flag. | |
| 157 | + * | |
| 158 | + * The first cut of this guard read `enabled` for all of them. Only | |
| 159 | + * `rum` and `critical-css` have that key — `images` is switched on PER | |
| 160 | + * FORMAT (`webp` / `avif`), so a site with conversion fully on was | |
| 161 | + * still told to buy image conversion, and the Hub kept drawing an | |
| 162 | + * Enable button that could never clear. (#187 review) | |
| 163 | + * | |
| 164 | + * A closure per id, receiving that module's EFFECTIVE options — schema | |
| 165 | + * defaults merged over the stored row, which is what the module actually | |
| 166 | + * runs on. Reading the raw row instead missed every setting still at its | |
| 167 | + * default, and Images defaults `webp` to true. (#187 QA round 2) | |
| 168 | + * | |
| 169 | + * @return array<string, callable(array):bool> | |
| 170 | + */ | |
| 171 | + private static function activity_probes(): array { | |
| 172 | + return array( | |
| 173 | + // Conversion is per format — either one means new uploads are | |
| 174 | + // being converted, which is the thing the suggestion sells. | |
| 175 | + 'webp-avif' => static function ( array $o ): bool { | |
| 176 | + return ! empty( $o['webp'] ) || ! empty( $o['avif'] ); | |
| 177 | + }, | |
| 178 | + ); | |
| 179 | + } | |
| 180 | + | |
| 181 | + /** | |
| 182 | + * Is the Pro module backing this suggestion already active? | |
| 183 | + * | |
| 184 | + * Resolved by module SLUG, never by referencing a Pro class: the audit | |
| 185 | + * runs on Free installs where those classes do not exist, and Free never | |
| 186 | + * names Pro. Settings_Manager returns an empty array for a slug that is | |
| 187 | + * not registered, so Free resolves to "not active" and keeps suggesting. | |
| 188 | + */ | |
| 189 | + private static function already_active( string $id ): bool { | |
| 190 | + $slug = self::BACKING_MODULE[ $id ] ?? ''; | |
| 191 | + if ( '' === $slug ) { | |
| 192 | + return false; | |
| 193 | + } | |
| 194 | + | |
| 195 | + /* | |
| 196 | + * Suppression is only honest while Pro is installed AND licensed. | |
| 197 | + * | |
| 198 | + * Deactivating Pro leaves its settings rows behind, and nothing | |
| 199 | + * clears them — Pro has no uninstall routine for them, so deleting | |
| 200 | + * the plugin does not help either. Reading those rows directly meant | |
| 201 | + * a Free-only site with the same history was silently never shown | |
| 202 | + * the RUM and Critical CSS suggestions again. An expired licence is | |
| 203 | + * the same shape with a sharper edge: the feature is gated OFF and | |
| 204 | + * genuinely not running, which is exactly the moment a renewal | |
| 205 | + * prompt is most useful. (#187 review) | |
| 206 | + * | |
| 207 | + * `xspeed_pro_state` is the signal Pro already reports to Free for | |
| 208 | + * the gated UI; it needs no Pro class reference here. | |
| 209 | + */ | |
| 210 | + if ( 'active' !== self::pro_state() ) { | |
| 211 | + return false; | |
| 212 | + } | |
| 213 | + | |
| 214 | + /* | |
| 215 | + * Read the module's EFFECTIVE settings, not its stored row. | |
| 216 | + * | |
| 217 | + * A raw get_option() sees only what someone has explicitly saved. A | |
| 218 | + * module's schema defaults are what it actually runs on until then — | |
| 219 | + * Pro's Images module defaults `webp` to true, so a site that bought | |
| 220 | + * Pro and never opened the Images panel is converting images while | |
| 221 | + * its option row has no `webp` key at all. The probe read false and | |
| 222 | + * the audit told that customer to buy image conversion: the exact | |
| 223 | + * complaint in #187, reproduced on a different feature. Opening the | |
| 224 | + * panel and pressing Save with no changes made the suggestion vanish, | |
| 225 | + * which is the tell that the check was reading the wrong thing rather | |
| 226 | + * than a genuine "off". (#187 QA round 2) | |
| 227 | + * | |
| 228 | + * Settings_Manager::get() merges schema defaults over the stored row. | |
| 229 | + * It returns array() for a module that is not registered, so on Free — | |
| 230 | + * where the Pro module does not exist — this still resolves to "not | |
| 231 | + * active" and every suggestion keeps firing. Safe to call here because | |
| 232 | + * the pro_state() gate above means Pro is loaded by this point. | |
| 233 | + */ | |
| 234 | + $opts = Settings_Manager::get( $slug ); | |
| 235 | + $probes = self::activity_probes(); | |
| 236 | + if ( isset( $probes[ $id ] ) ) { | |
| 237 | + return (bool) $probes[ $id ]( $opts ); | |
| 238 | + } | |
| 239 | + | |
| 240 | + return ! empty( $opts['enabled'] ); | |
| 241 | + } | |
| 242 | + | |
| 243 | + /** | |
| 244 | + * Suggestions contributed by an add-on, normalised to the native shape. | |
| 245 | + * | |
| 246 | + * The rules in run() only know what the Free engine can see: options and | |
| 247 | + * cache counters. An add-on that owns a feature knows things about it that | |
| 248 | + * no option records — that a generator has never once succeeded, that a | |
| 249 | + * conversion is producing files bigger than the ones it replaces — and | |
| 250 | + * before this filter it had nowhere to say so. The finding stayed inside | |
| 251 | + * that add-on's own panel, and `get_pro_audit`, which is what an agent | |
| 252 | + * reads when it asks "what is wrong with this site", never heard about it. | |
| 253 | + * | |
| 254 | + * DELIBERATELY NOT GATED BY already_active(). That guard exists to stop the | |
| 255 | + * audit nagging someone to buy a feature they already own (#187), which is | |
| 256 | + * an upsell concern: an upsell for a feature that is already on can never | |
| 257 | + * be acted on. A contribution is the opposite kind of message — it comes | |
| 258 | + * FROM the feature, and the feature has to be switched on to have anything | |
| 259 | + * to report. Inheriting the suppression would silence exactly the case | |
| 260 | + * worth hearing: a feature that is on and misbehaving. Do not "tidy up" by | |
| 261 | + * routing this through already_active(). (xspeed-pro#86) | |
| 262 | + * | |
| 263 | + * Contributors may only ADD. The filter is seeded with an empty array and | |
| 264 | + * the native list is passed as read-only context, so nothing a contributor | |
| 265 | + * returns can delete or rewrite a native suggestion. Contributions are | |
| 266 | + * appended after the native rules and then go through the same dedupe and | |
| 267 | + * severity sort, so a contribution takes over a native id only by being | |
| 268 | + * strictly more severe — never by merely arriving later. | |
| 269 | + * | |
| 270 | + * @param array<int,array<string,mixed>> $native Suggestions the native | |
| 271 | + * rules produced, as context. | |
| 272 | + * @return array<int,array{id:string,severity:string,reason:string,fact?:string}> | |
| 273 | + */ | |
| 274 | + private static function contributed( array $native ): array { | |
| 275 | + /** | |
| 276 | + * Filter: xspeed_pro_audit_suggestions | |
| 277 | + * | |
| 278 | + * Extra suggestions to append to the audit. Seeded with an empty | |
| 279 | + * array — append your own entries and return the array; the native | |
| 280 | + * suggestions are passed separately as read-only context, so nothing | |
| 281 | + * returned here can remove or rewrite one. | |
| 282 | + * | |
| 283 | + * Each entry: id (string, required — a feature slug; matches a key in | |
| 284 | + * the React PRO_FEATURES catalog when one exists, otherwise the id | |
| 285 | + * itself is shown as the title), severity ('high'|'med'|'low', | |
| 286 | + * defaults to 'low'), reason (string, required — one sentence, | |
| 287 | + * already-baked numbers, no markup), fact (string, optional — a short | |
| 288 | + * inline stat for the card's chip). Anything else is dropped. | |
| 289 | + * | |
| 290 | + * @param array $suggestions Contributed suggestions (empty on entry). | |
| 291 | + * @param array $native The native suggestions, as context. | |
| 292 | + */ | |
| 293 | + try { | |
| 294 | + $raw = apply_filters( 'xspeed_pro_audit_suggestions', array(), $native ); | |
| 295 | + } catch ( \Throwable $e ) { | |
| 296 | + // A contributor that throws costs the audit its contributions, | |
| 297 | + // not the audit. Every native finding is already computed by the | |
| 298 | + // time this runs, and the audit is what the dashboard card and | |
| 299 | + // the MCP `get_pro_audit` tool both read — a seam that lets a | |
| 300 | + // broken add-on take those down is a liability to the thing it | |
| 301 | + // extends. | |
| 302 | + // | |
| 303 | + // The whole round is lost, not just the thrower's entry: the | |
| 304 | + // throw unwinds through apply_filters(), so a well-behaved | |
| 305 | + // contributor that ran earlier has no partial result left to | |
| 306 | + // salvage. Nothing to do about that from out here, but it is | |
| 307 | + // the reason this says "contributions" and not "its | |
| 308 | + // contribution". | |
| 309 | + // | |
| 310 | + // core's apply_filters() pops $wp_current_filter AFTER the | |
| 311 | + // callbacks return, so a throw leaves our hook name on the | |
| 312 | + // stack: current_filter() would keep answering | |
| 313 | + // 'xspeed_pro_audit_suggestions' for the rest of the request, | |
| 314 | + // and core's own lazy-loading branches on that. Pop it back, | |
| 315 | + // and only if it is still ours to pop. (WP_Hook's nesting_level | |
| 316 | + // leaks the same way and cannot be reached from here; it is | |
| 317 | + // scoped to this one hook, which we are done with.) | |
| 318 | + if ( isset( $GLOBALS['wp_current_filter'] ) | |
| 319 | + && is_array( $GLOBALS['wp_current_filter'] ) | |
| 320 | + && end( $GLOBALS['wp_current_filter'] ) === 'xspeed_pro_audit_suggestions' ) { | |
| 321 | + array_pop( $GLOBALS['wp_current_filter'] ); | |
| 322 | + } | |
| 323 | + if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { | |
| 324 | + // Swallowing a fatal without a word makes a broken add-on | |
| 325 | + // indistinguishable from one with nothing to report. | |
| 326 | + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log | |
| 327 | + error_log( '[xspeed] xspeed_pro_audit_suggestions contributor threw: ' . $e->getMessage() ); | |
| 328 | + } | |
| 329 | + return array(); | |
| 330 | + } | |
| 331 | + if ( ! is_array( $raw ) ) { | |
| 332 | + return array(); | |
| 333 | + } | |
| 334 | + | |
| 335 | + $out = array(); | |
| 336 | + foreach ( $raw as $row ) { | |
| 337 | + if ( count( $out ) >= self::MAX_CONTRIBUTED ) { | |
| 338 | + break; | |
| 339 | + } | |
| 340 | + $clean = self::normalize_suggestion( $row ); | |
| 341 | + if ( null !== $clean ) { | |
| 342 | + $out[] = $clean; | |
| 343 | + } | |
| 344 | + } | |
| 345 | + return $out; | |
| 346 | + } | |
| 347 | + | |
| 348 | + /** | |
| 349 | + * Coerce one contributed entry into the exact shape run() emits, or drop it. | |
| 350 | + * | |
| 351 | + * Everything downstream — the dedupe, the severity sort, the React card, | |
| 352 | + * the MCP tool's response — is written against `{id, severity, reason, | |
| 353 | + * fact?}` and nothing re-checks it. An unknown severity alone is enough to | |
| 354 | + * break the panel, which indexes a style map by it. So this is a whitelist, | |
| 355 | + * not a merge: unrecognised keys are dropped rather than passed through, and | |
| 356 | + * an entry that can't supply an id and a reason is dropped whole. A missing | |
| 357 | + * or invalid severity is not fatal, but it settles at 'low' — a contributor | |
| 358 | + * who won't say how bad it is doesn't get to outrank anyone. | |
| 359 | + * | |
| 360 | + * @param mixed $row Whatever the filter returned in this slot. | |
| 361 | + * @return array{id:string,severity:string,reason:string,fact?:string}|null | |
| 362 | + */ | |
| 363 | + private static function normalize_suggestion( $row ): ?array { | |
| 364 | + if ( ! is_array( $row ) ) { | |
| 365 | + return null; | |
| 366 | + } | |
| 367 | + | |
| 368 | + $id = sanitize_key( self::as_text( $row['id'] ?? null ) ); | |
| 369 | + if ( '' === $id ) { | |
| 370 | + return null; | |
| 371 | + } | |
| 372 | + | |
| 373 | + $reason = self::as_text( $row['reason'] ?? null ); | |
| 374 | + $reason = trim( (string) sanitize_text_field( $reason ) ); | |
| 375 | + if ( '' === $reason ) { | |
| 376 | + return null; | |
| 377 | + } | |
| 378 | + | |
| 379 | + $severity = self::as_text( $row['severity'] ?? null ); | |
| 380 | + if ( ! in_array( $severity, array( self::SEVERITY_HIGH, self::SEVERITY_MED, self::SEVERITY_LOW ), true ) ) { | |
| 381 | + $severity = self::SEVERITY_LOW; | |
| 382 | + } | |
| 383 | + | |
| 384 | + $clean = array( | |
| 385 | + 'id' => $id, | |
| 386 | + 'severity' => $severity, | |
| 387 | + 'reason' => self::clamp( $reason, self::MAX_REASON_LEN ), | |
| 388 | + ); | |
| 389 | + | |
| 390 | + $fact = trim( (string) sanitize_text_field( self::as_text( $row['fact'] ?? null ) ) ); | |
| 391 | + if ( '' !== $fact ) { | |
| 392 | + $clean['fact'] = self::clamp( $fact, self::MAX_FACT_LEN ); | |
| 393 | + } | |
| 394 | + | |
| 395 | + return $clean; | |
| 396 | + } | |
| 397 | + | |
| 398 | + /** | |
| 399 | + * A string, or '' for anything that isn't honestly one. | |
| 400 | + * | |
| 401 | + * Booleans are excluded on purpose: `(string) true` is "1", which would | |
| 402 | + * sail through an is_scalar() check and land a suggestion whose reason | |
| 403 | + * reads "1". | |
| 404 | + * | |
| 405 | + * @param mixed $value Raw value from a contributed entry. | |
| 406 | + */ | |
| 407 | + private static function as_text( $value ): string { | |
| 408 | + if ( is_string( $value ) ) { | |
| 409 | + return $value; | |
| 410 | + } | |
| 411 | + if ( is_int( $value ) || is_float( $value ) ) { | |
| 412 | + return (string) $value; | |
| 413 | + } | |
| 414 | + return ''; | |
| 415 | + } | |
| 416 | + | |
| 417 | + /** | |
| 418 | + * Trim to a hard character budget, ellipsis included in the budget. | |
| 419 | + * | |
| 420 | + * @param string $text Text to bound. | |
| 421 | + * @param int $limit Maximum length of the result. | |
| 422 | + */ | |
| 423 | + private static function clamp( string $text, int $limit ): string { | |
| 424 | + if ( function_exists( 'mb_strlen' ) && function_exists( 'mb_substr' ) ) { | |
| 425 | + if ( mb_strlen( $text ) <= $limit ) { | |
| 426 | + return $text; | |
| 427 | + } | |
| 428 | + return rtrim( mb_substr( $text, 0, $limit - 1 ) ) . '…'; | |
| 429 | + } | |
| 430 | + return self::clamp_without_mbstring( $text, $limit ); | |
| 431 | + } | |
| 432 | + | |
| 433 | + /** | |
| 434 | + * clamp() on a site with no mbstring. | |
| 435 | + * | |
| 436 | + * strlen()/substr() count BYTES, and using them here got the budget wrong | |
| 437 | + * in both directions at once. Too tight: 400 bytes of accented Latin is | |
| 438 | + * under 200 characters, so a reason well inside its allowance came back | |
| 439 | + * truncated. And unsafe: a byte cut can land inside a character, and | |
| 440 | + * wp_json_encode() answers false for the WHOLE response rather than | |
| 441 | + * mangling one word — the client loses every finding in the audit. | |
| 442 | + * | |
| 443 | + * PCRE counts characters in `/u` mode without mbstring, so the budget | |
| 444 | + * stays a character budget. Both patterns are bounded by `$limit`, so | |
| 445 | + * neither walks a long string or builds an array of it. | |
| 446 | + * | |
| 447 | + * @param string $text Text to bound. | |
| 448 | + * @param int $limit Maximum length of the result. | |
| 449 | + */ | |
| 450 | + private static function clamp_without_mbstring( string $text, int $limit ): string { | |
| 451 | + $limit = max( 1, $limit ); | |
| 452 | + | |
| 453 | + // preg_match() returns false — not 0 — when the subject is not valid | |
| 454 | + // UTF-8, which is how the byte fallback below is reached. | |
| 455 | + $within = preg_match( '/^.{0,' . $limit . '}$/us', $text ); | |
| 456 | + if ( 1 === $within ) { | |
| 457 | + return $text; | |
| 458 | + } | |
| 459 | + if ( 0 === $within && 1 === preg_match( '/^.{0,' . ( $limit - 1 ) . '}/us', $text, $m ) ) { | |
| 460 | + return rtrim( $m[0] ) . '…'; | |
| 461 | + } | |
| 462 | + | |
| 463 | + // Not valid UTF-8 to begin with — a contributor sent bytes we cannot | |
| 464 | + // count. Bytes are all there is, but the result still has to encode. | |
| 465 | + if ( strlen( $text ) <= $limit ) { | |
| 466 | + return $text; | |
| 467 | + } | |
| 468 | + return rtrim( self::whole_characters( substr( $text, 0, $limit - 1 ) ) ) . '…'; | |
| 469 | + } | |
| 470 | + | |
| 471 | + /** | |
| 472 | + * Drop a trailing partial UTF-8 character. | |
| 473 | + * | |
| 474 | + * A byte cut can land inside a multibyte character and leave a dangling | |
| 475 | + * fragment. That makes the string invalid UTF-8, and wp_json_encode() | |
| 476 | + * answers false for the WHOLE response — the client loses every finding, | |
| 477 | + * not one accented word. At most three bytes come off. | |
| 478 | + * | |
| 479 | + * @param string $bytes Byte-cut text. | |
| 480 | + */ | |
| 481 | + private static function whole_characters( string $bytes ): string { | |
| 482 | + // `//u` is an empty pattern with the UTF-8 modifier: it matches | |
| 483 | + // anything, and fails outright when the subject is not valid UTF-8. | |
| 484 | + while ( '' !== $bytes && 1 !== preg_match( '//u', $bytes ) ) { | |
| 485 | + $bytes = substr( $bytes, 0, -1 ); | |
| 486 | + } | |
| 487 | + return $bytes; | |
| 488 | + } | |
| 489 | + | |
| 490 | + /** | |
| 491 | + * Pro's own report of its licence state: 'not_installed' | 'unlicensed' | |
| 492 | + * | 'active'. Mirrors Admin::pro_state(), which is private to that | |
| 493 | + * class; only 'active' means Pro's features are actually running. | |
| 494 | + */ | |
| 495 | + private static function pro_state(): string { | |
| 496 | + $present = class_exists( '\\XSpeed\\Tier_Registry' ) && Tier_Registry::pro_active(); | |
| 497 | + $default = $present ? 'active' : 'not_installed'; | |
| 498 | + | |
| 499 | + /** This filter is documented in includes/class-admin.php */ | |
| 500 | + $state = (string) apply_filters( 'xspeed_pro_state', $default ); | |
| 501 | + | |
| 502 | + return in_array( $state, array( 'not_installed', 'unlicensed', 'active' ), true ) ? $state : $default; | |
| 503 | + } | |
| 504 | + | |
| 505 | + /** | |
| 91 | 506 | * @param array|null $totals_override See snapshot(). Production |
| 92 | 507 | * callers pass nothing. |
| 93 | 508 | * |
| 94 | 509 | * @return array<int,array{id:string,severity:string,reason:string,fact?:string}> |
| @@ -99,8 +514,17 @@ | ||
| 99 | 514 | |
| 100 | 515 | // Rule 1 — Cloudflare connected but APO not in use. |
| 101 | 516 | // High signal: user already pays the Cloudflare overhead, APO |
| 102 | 517 | // is the highest-leverage Pro feature they can flip on next. |
| 518 | + // | |
| 519 | + // Deliberately NOT guarded by already_active(): APO's real state | |
| 520 | + // lives at Cloudflare, not in an option here, and finding out means | |
| 521 | + // a live API call — which this audit runs on every dashboard load, | |
| 522 | + // including Free installs with no credentials. The guard used to be | |
| 523 | + // called here anyway; it could never fire (no BACKING_MODULE entry), | |
| 524 | + // so it produced the right outcome by accident while reading as | |
| 525 | + // though the case were handled. See the note beside BACKING_MODULE. | |
| 526 | + // (#187 QA round 2) | |
| 103 | 527 | if ( ! empty( $s['cloudflare']['enabled'] ) ) { |
| 104 | 528 | $out[] = array( |
| 105 | 529 | 'id' => 'cloudflare-apo', |
| 106 | 530 | 'severity' => self::SEVERITY_HIGH, |
| @@ -110,10 +534,17 @@ | ||
| 110 | 534 | } |
| 111 | 535 | |
| 112 | 536 | // Rule 2 — Low cache hit ratio with meaningful traffic. |
| 113 | 537 | // "Meaningful" = > 50 hits over 24h; below that the ratio is |
| 114 | - // statistical noise and we'd suggest based on bad data. | |
| 115 | - if ( $s['totals_24h']['total'] >= 50 && $s['totals_24h']['ratio'] < 0.5 ) { | |
| 538 | + // statistical noise and we'd suggest based on bad data. The ratio is | |
| 539 | + // now computed over real traffic only (404s + bots excluded, #118), and | |
| 540 | + // we skip it entirely when an edge cache fronts the origin — behind | |
| 541 | + // Cloudflare a low origin ratio means hits are served at the edge, not | |
| 542 | + // that the cache is failing, so firing a "your cache is bad" upsell off | |
| 543 | + // it is selling against a measurement artefact. | |
| 544 | + if ( empty( $s['edge_cache'] ) | |
| 545 | + && $s['totals_24h']['total'] >= 50 | |
| 546 | + && $s['totals_24h']['ratio'] < 0.5 ) { | |
| 116 | 547 | $pct = (int) round( $s['totals_24h']['ratio'] * 100 ); |
| 117 | 548 | $out[] = array( |
| 118 | 549 | 'id' => 'recommendations', |
| 119 | 550 | 'severity' => self::SEVERITY_HIGH, |
| @@ -126,9 +557,9 @@ | ||
| 126 | 557 | } |
| 127 | 558 | |
| 128 | 559 | // Rule 3 — Lazy-load enabled but no auto WebP/AVIF. |
| 129 | 560 | // User cares about images (lazy on) → next gain is format. |
| 130 | - if ( ! empty( $s['lazy']['lazy_images'] ) ) { | |
| 561 | + if ( ! empty( $s['lazy']['lazy_images'] ) && ! self::already_active( 'webp-avif' ) ) { | |
| 131 | 562 | $out[] = array( |
| 132 | 563 | 'id' => 'webp-avif', |
| 133 | 564 | 'severity' => self::SEVERITY_MED, |
| 134 | 565 | 'reason' => 'Images are lazy-loaded. Pro auto-converts new JPEG/PNG uploads to WebP and AVIF — typically 25-35% smaller at the same visual quality.', |
| @@ -137,9 +568,9 @@ | ||
| 137 | 568 | |
| 138 | 569 | // Rule 4 — High traffic without RUM data. |
| 139 | 570 | // Real-user metrics matter more than synthetic Lighthouse when |
| 140 | 571 | // the site has actual visitors. |
| 141 | - if ( $s['totals_24h']['total'] >= 100 ) { | |
| 572 | + if ( $s['totals_24h']['total'] >= 100 && ! self::already_active( 'rum' ) ) { | |
| 142 | 573 | $views = number_format( $s['totals_24h']['total'] ); |
| 143 | 574 | $out[] = array( |
| 144 | 575 | 'id' => 'rum', |
| 145 | 576 | 'severity' => self::SEVERITY_MED, |
| @@ -152,9 +583,10 @@ | ||
| 152 | 583 | } |
| 153 | 584 | |
| 154 | 585 | // Rule 5 — HTML minify on but JS minify off (theme-safe stance). |
| 155 | 586 | // Suggest Critical CSS as the next gain that doesn't touch JS. |
| 156 | - if ( ! empty( $s['minify']['minify_html'] ) && empty( $s['minify']['minify_js'] ) ) { | |
| 587 | + if ( ! empty( $s['minify']['minify_html'] ) && empty( $s['minify']['minify_js'] ) | |
| 588 | + && ! self::already_active( 'critical-css' ) ) { | |
| 157 | 589 | $out[] = array( |
| 158 | 590 | 'id' => 'critical-css', |
| 159 | 591 | 'severity' => self::SEVERITY_MED, |
| 160 | 592 | 'reason' => 'JS minify is off (good — high theme-conflict risk). Pro Critical CSS delivers similar first-paint gains without touching JavaScript.', |
| @@ -196,8 +628,13 @@ | ||
| 196 | 628 | ), |
| 197 | 629 | 'fact' => $enabled . ' modules', |
| 198 | 630 | ); |
| 199 | 631 | } |
| 632 | + | |
| 633 | + // Add-on contributions. Collected after the native rules and before | |
| 634 | + // the fallback: a site whose only real finding comes from an add-on | |
| 635 | + // should get that finding, not the generic filler underneath it. | |
| 636 | + $out = array_merge( $out, self::contributed( $out ) ); | |
| 200 | 637 | |
| 201 | 638 | // Fallback — never return an empty audit. Analytics is the |
| 202 | 639 | // safe always-relevant suggestion (every site has cache |
| 203 | 640 | // activity to chart). |