| @@ -40,8 +40,36 @@ | ||
| 40 | 40 | */ |
| 41 | 41 | class Site_Identity_Manager extends Abstract_SEO_Manager { |
| 42 | 42 | |
| 43 | 43 | /** |
| 44 | + * The per-context title formats as ThinkRank ships them. | |
| 45 | + * | |
| 46 | + * These are not in get_default_settings(): the admin screen seeds them on | |
| 47 | + * first save, so on a real install they are stored values, indistinguishable | |
| 48 | + * from a template the user typed. The migration needs to tell those two | |
| 49 | + * apart — it may overwrite a shipped default with an imported template, and | |
| 50 | + * must never overwrite a choice the user made — so this is the record of | |
| 51 | + * what "untouched" looks like. | |
| 52 | + * | |
| 53 | + * Keep in step with getDefaultSettings() in | |
| 54 | + * src/admin/components/essential-seo/SiteIdentityTab.js. SiteIdentityTitleFormatDefaultsTest | |
| 55 | + * fails when the two drift. | |
| 56 | + * | |
| 57 | + * @since 2.8.0 | |
| 58 | + * @var array<string, string> | |
| 59 | + */ | |
| 60 | + public const TITLE_FORMAT_DEFAULTS = [ | |
| 61 | + 'homepage_title' => '%site_title% %sep% %site_description%', | |
| 62 | + 'post_title' => '%post_title% %sep% %site_title%', | |
| 63 | + 'page_title' => '%page_title% %sep% %site_title%', | |
| 64 | + 'category_title' => '%category_title% %sep% %site_title%', | |
| 65 | + 'tag_title' => '%tag_title% %sep% %site_title%', | |
| 66 | + 'author_title' => '%author_name% %sep% %site_title%', | |
| 67 | + 'search_title' => 'Search Results for "%search_term%" %sep% %site_title%', | |
| 68 | + 'archive_title' => '%archive_title% %sep% %site_title%', | |
| 69 | + ]; | |
| 70 | + | |
| 71 | + /** | |
| 44 | 72 | * WordPress filesystem instance |
| 45 | 73 | * |
| 46 | 74 | * @since 1.0.0 |
| 47 | 75 | * @var \WP_Filesystem_Base|null |
| @@ -290,8 +318,36 @@ | ||
| 290 | 318 | * @var bool |
| 291 | 319 | */ |
| 292 | 320 | private static bool $icon_sizes_listener_registered = false; |
| 293 | 321 | |
| 322 | + /** | |
| 323 | + * Whether the robots.txt resync listener is registered for this request. | |
| 324 | + * | |
| 325 | + * @since 2.14.0 | |
| 326 | + * @var bool | |
| 327 | + */ | |
| 328 | + private static bool $robots_sync_listener_registered = false; | |
| 329 | + | |
| 330 | + /** | |
| 331 | + * Flag set when a plugin change may have altered the sitemap set. | |
| 332 | + * | |
| 333 | + * @since 2.14.0 | |
| 334 | + * @var string | |
| 335 | + */ | |
| 336 | + public const ROBOTS_RESYNC_OPTION = 'thinkrank_robots_txt_resync_pending'; | |
| 337 | + | |
| 338 | + /** | |
| 339 | + * Flag set when a plugin change may have altered the sitemap index. | |
| 340 | + * | |
| 341 | + * Separate from ROBOTS_RESYNC_OPTION because the two files exist | |
| 342 | + * independently: that flag is only set when a physical robots.txt exists, | |
| 343 | + * and the sitemap index needs rebuilding whether or not it does. | |
| 344 | + * | |
| 345 | + * @since 2.15.0 | |
| 346 | + * @var string | |
| 347 | + */ | |
| 348 | + public const SITEMAP_RESYNC_OPTION = 'thinkrank_sitemap_contributors_changed'; | |
| 349 | + | |
| 294 | 350 | public function __construct() { |
| 295 | 351 | parent::__construct('site_identity'); |
| 296 | 352 | |
| 297 | 353 | if (!self::$icon_sizes_listener_registered) { |
| @@ -300,11 +356,213 @@ | ||
| 300 | 356 | // Admin only: resizing is not front-end work, and admin traffic is |
| 301 | 357 | // enough to run a one-time backfill promptly. |
| 302 | 358 | add_action('admin_init', [self::class, 'maybe_backfill_icon_sizes']); |
| 303 | 359 | } |
| 360 | + | |
| 361 | + if (!self::$robots_sync_listener_registered) { | |
| 362 | + self::$robots_sync_listener_registered = true; | |
| 363 | + | |
| 364 | + // A physical robots.txt bypasses PHP entirely, so composing the | |
| 365 | + // Sitemap block at render time fixes the served output only on | |
| 366 | + // sites with no file. Activating or deactivating a sitemap | |
| 367 | + // contributor changes the set, and until #835 nothing rewrote the | |
| 368 | + // file: the deactivated plugin's sitemap stayed advertised, serving | |
| 369 | + // HTML to anything that followed it. | |
| 370 | + add_action('activated_plugin', [self::class, 'flag_robots_txt_resync']); | |
| 371 | + add_action('deactivated_plugin', [self::class, 'flag_robots_txt_resync']); | |
| 372 | + add_action('init', [self::class, 'maybe_resync_robots_txt'], 99); | |
| 373 | + | |
| 374 | + // The sitemap index is a second static file listing the same | |
| 375 | + // contributors, with its own rebuild path. #835 / #859 resynced | |
| 376 | + // robots.txt only, so after Pro was deactivated the index kept | |
| 377 | + // advertising news-sitemap.xml, which then served the home page | |
| 378 | + // as HTML (#920). | |
| 379 | + add_action('activated_plugin', [self::class, 'flag_sitemap_resync']); | |
| 380 | + add_action('deactivated_plugin', [self::class, 'flag_sitemap_resync']); | |
| 381 | + add_action('init', [self::class, 'maybe_resync_sitemap'], 99); | |
| 382 | + } | |
| 304 | 383 | } |
| 305 | 384 | |
| 306 | 385 | /** |
| 386 | + * Note that the set of sitemap contributors may have changed. | |
| 387 | + * | |
| 388 | + * Deliberately unconditional about which plugin: a contributor is anything | |
| 389 | + * hooking `thinkrank_additional_sitemaps`, which is resolved at runtime and | |
| 390 | + * cannot be inspected for a plugin that is on its way out. | |
| 391 | + * | |
| 392 | + * The rewrite is not done here. `deactivated_plugin` fires inside the | |
| 393 | + * request that deactivated it, while that plugin's filters are still | |
| 394 | + * attached, so rendering now still sees the sitemap that is going away — | |
| 395 | + * measured, not assumed: the first version of this fix wrote the | |
| 396 | + * deactivated plugin's sitemap straight back into the file. The next | |
| 397 | + * request has the real plugin set loaded, so the work waits for it. | |
| 398 | + * | |
| 399 | + * @since 2.14.0 | |
| 400 | + * @return void | |
| 401 | + */ | |
| 402 | + public static function flag_robots_txt_resync(): void { | |
| 403 | + if (!file_exists(ABSPATH . 'robots.txt')) { | |
| 404 | + return; | |
| 405 | + } | |
| 406 | + | |
| 407 | + update_option(self::ROBOTS_RESYNC_OPTION, 1, false); | |
| 408 | + } | |
| 409 | + | |
| 410 | + /** | |
| 411 | + * Rewrite the physical robots.txt once, on the request after a change. | |
| 412 | + * | |
| 413 | + * @since 2.14.0 | |
| 414 | + * @return void | |
| 415 | + */ | |
| 416 | + public static function maybe_resync_robots_txt(): void { | |
| 417 | + if (!get_option(self::ROBOTS_RESYNC_OPTION)) { | |
| 418 | + return; | |
| 419 | + } | |
| 420 | + | |
| 421 | + // Cleared first, so a render that fatals cannot retry on every request | |
| 422 | + // for the rest of the site's life. | |
| 423 | + delete_option(self::ROBOTS_RESYNC_OPTION); | |
| 424 | + | |
| 425 | + if (!file_exists(ABSPATH . 'robots.txt')) { | |
| 426 | + return; | |
| 427 | + } | |
| 428 | + | |
| 429 | + (new self())->sync_robots_txt_file(); | |
| 430 | + } | |
| 431 | + | |
| 432 | + /** | |
| 433 | + * Note that the set of sitemap contributors may have changed. | |
| 434 | + * | |
| 435 | + * Unconditional, unlike flag_robots_txt_resync(): the sitemap files exist | |
| 436 | + * whether or not robots.txt does. The rebuild waits for the next request | |
| 437 | + * for the same reason as the robots.txt one, since `deactivated_plugin` | |
| 438 | + * still runs with the outgoing plugin's `thinkrank_additional_sitemaps` | |
| 439 | + * callback attached. | |
| 440 | + * | |
| 441 | + * @since 2.15.0 | |
| 442 | + * @return void | |
| 443 | + */ | |
| 444 | + public static function flag_sitemap_resync(): void { | |
| 445 | + update_option(self::SITEMAP_RESYNC_OPTION, 1, false); | |
| 446 | + } | |
| 447 | + | |
| 448 | + /** | |
| 449 | + * Queue a sitemap rebuild once, on the request after a contributor change. | |
| 450 | + * | |
| 451 | + * Goes through schedule_regeneration(), the debounced and lock-protected | |
| 452 | + * path a sitemap settings save uses, so a burst of plugin changes (a bulk | |
| 453 | + * deactivate, say) still produces one rebuild. That path also drops the | |
| 454 | + * cached dynamic documents, so sites serving the sitemap from PHP drop the | |
| 455 | + * entry as well. | |
| 456 | + * | |
| 457 | + * @since 2.15.0 | |
| 458 | + * @return void | |
| 459 | + */ | |
| 460 | + public static function maybe_resync_sitemap(): void { | |
| 461 | + if (!get_option(self::SITEMAP_RESYNC_OPTION)) { | |
| 462 | + return; | |
| 463 | + } | |
| 464 | + | |
| 465 | + // Cleared first, so a rebuild that fatals cannot be retried on every | |
| 466 | + // request for the rest of the site's life. | |
| 467 | + delete_option(self::SITEMAP_RESYNC_OPTION); | |
| 468 | + | |
| 469 | + $generator = new Sitemap_Generator(false); | |
| 470 | + $settings = $generator->get_settings('site'); | |
| 471 | + | |
| 472 | + // A disabled sitemap has no files to correct. Enabling it later builds | |
| 473 | + // from the contributors present at that time. | |
| 474 | + if (empty($settings['enabled'])) { | |
| 475 | + return; | |
| 476 | + } | |
| 477 | + | |
| 478 | + $generator->schedule_regeneration(); | |
| 479 | + } | |
| 480 | + | |
| 481 | + /** | |
| 482 | + * Save settings, then refresh what a new canonical scheme invalidates. | |
| 483 | + * | |
| 484 | + * The static sitemap files are written with the scheme in force when they | |
| 485 | + * were built, and nothing else rebuilds them until a post or term changes. | |
| 486 | + * So a change of scheme left every `<loc>` on the old one while canonical | |
| 487 | + * and og:url had already moved (#736). Every writer (the settings route, | |
| 488 | + * the robots route, the MCP abilities, an import) lands here. | |
| 489 | + * | |
| 490 | + * @since 2.7.0 | |
| 491 | + * | |
| 492 | + * @param string $context_type Context type. | |
| 493 | + * @param int|null $context_id Context ID. | |
| 494 | + * @param array $settings Settings to save. | |
| 495 | + * @return bool | |
| 496 | + */ | |
| 497 | + public function save_settings(string $context_type, ?int $context_id, array $settings): bool { | |
| 498 | + if (!self::touches_canonical_scheme($context_type, $context_id, $settings)) { | |
| 499 | + return parent::save_settings($context_type, $context_id, $settings); | |
| 500 | + } | |
| 501 | + | |
| 502 | + $before = Url_Scheme::preference(); | |
| 503 | + $saved = parent::save_settings($context_type, $context_id, $settings); | |
| 504 | + | |
| 505 | + if ($saved) { | |
| 506 | + $this->on_canonical_scheme_saved($before); | |
| 507 | + } | |
| 508 | + | |
| 509 | + return $saved; | |
| 510 | + } | |
| 511 | + | |
| 512 | + /** | |
| 513 | + * Whether a save can change the site-wide canonical scheme. | |
| 514 | + * | |
| 515 | + * @since 2.7.0 | |
| 516 | + * | |
| 517 | + * @param string $context_type Context type. | |
| 518 | + * @param int|null $context_id Context ID. | |
| 519 | + * @param array $settings Settings being saved. | |
| 520 | + * @return bool | |
| 521 | + */ | |
| 522 | + public static function touches_canonical_scheme(string $context_type, ?int $context_id, array $settings): bool { | |
| 523 | + return 'site' === sanitize_key($context_type) | |
| 524 | + && empty($context_id) | |
| 525 | + && array_key_exists('canonical_scheme', $settings); | |
| 526 | + } | |
| 527 | + | |
| 528 | + /** | |
| 529 | + * Rebuild the static sitemaps when the effective scheme changed. | |
| 530 | + * | |
| 531 | + * Compares the effective preference, filter included, so a site whose | |
| 532 | + * scheme is pinned by `thinkrank_canonical_scheme` does not rebuild on a | |
| 533 | + * stored value that changes nothing it publishes. | |
| 534 | + * | |
| 535 | + * @since 2.7.0 | |
| 536 | + * | |
| 537 | + * @param string $before Effective scheme before the save. | |
| 538 | + * @return void | |
| 539 | + */ | |
| 540 | + protected function on_canonical_scheme_saved(string $before): void { | |
| 541 | + // The preference is cached for the request; the save just changed it. | |
| 542 | + Url_Scheme::reset(); | |
| 543 | + | |
| 544 | + if (Url_Scheme::preference() === $before) { | |
| 545 | + return; | |
| 546 | + } | |
| 547 | + | |
| 548 | + $this->schedule_sitemap_rebuild(); | |
| 549 | + } | |
| 550 | + | |
| 551 | + /** | |
| 552 | + * Queue a settings-driven sitemap rebuild. | |
| 553 | + * | |
| 554 | + * Debounced and run after the response, like any other settings change | |
| 555 | + * that alters what the sitemap publishes. | |
| 556 | + * | |
| 557 | + * @since 2.7.0 | |
| 558 | + * @return void | |
| 559 | + */ | |
| 560 | + protected function schedule_sitemap_rebuild(): void { | |
| 561 | + (new Sitemap_Generator(false))->schedule_regeneration(); | |
| 562 | + } | |
| 563 | + | |
| 564 | + /** | |
| 307 | 565 | * Build the icon derivatives for a newly chosen favicon. |
| 308 | 566 | * |
| 309 | 567 | * Runs on save, which is the only moment the choice changes and the only |
| 310 | 568 | * place image work belongs — resolving a size on the front end must stay a |
| @@ -397,11 +655,12 @@ | ||
| 397 | 655 | * Attachment ID behind a configured icon URL, or 0 when it is not ours. |
| 398 | 656 | * |
| 399 | 657 | * attachment_url_to_postid() matches _wp_attached_file, which holds the |
| 400 | 658 | * ORIGINAL upload path, so the URL of a generated derivative |
| 401 | - * (`logo-512.png`) returns 0 — and that is exactly what the media picker | |
| 402 | - * hands back when the user chooses a size. Strip the dimension suffix and | |
| 403 | - * try the original once. | |
| 659 | + * (`logo-512x512.png`) returns 0 — and that is exactly what the media | |
| 660 | + * picker hands back when the user chooses a size. Attachment_Lookup falls | |
| 661 | + * back to the original behind it; the fallback started here and moved | |
| 662 | + * there when every other image lookup turned out to need it (#847). | |
| 404 | 663 | * |
| 405 | 664 | * Shared with SEO_Manager's site-icon filter so both sides of the feature |
| 406 | 665 | * agree on which attachment a configured URL means. |
| 407 | 666 | * |
| @@ -410,21 +669,9 @@ | ||
| 410 | 669 | * @param string $url Configured icon URL. |
| 411 | 670 | * @return int Attachment ID, or 0. |
| 412 | 671 | */ |
| 413 | 672 | public static function icon_attachment_id(string $url): int { |
| 414 | - $attachment_id = (int) attachment_url_to_postid($url); | |
| 415 | - | |
| 416 | - if ($attachment_id) { | |
| 417 | - return $attachment_id; | |
| 418 | - } | |
| 419 | - | |
| 420 | - $original = preg_replace('/-\d+x\d+(?=\.[a-zA-Z0-9]+$)/', '', $url); | |
| 421 | - | |
| 422 | - if (is_string($original) && $original !== $url) { | |
| 423 | - return (int) attachment_url_to_postid($original); | |
| 424 | - } | |
| 425 | - | |
| 426 | - return 0; | |
| 673 | + return Attachment_Lookup::id_from_url($url); | |
| 427 | 674 | } |
| 428 | 675 | |
| 429 | 676 | /** |
| 430 | 677 | * Which ICON_SIZES derivatives this attachment still needs. |
| @@ -714,9 +961,20 @@ | ||
| 714 | 961 | // here rather than stored: the textarea holds the user's body, with |
| 715 | 962 | // the fenced block stripped out of every read and re-applied on every |
| 716 | 963 | // render. A site-wide block already disallows everyone, so adding the |
| 717 | 964 | // per-agent group there would be noise restating the same refusal. |
| 965 | + // Composed here rather than read from storage, for the same reason as | |
| 966 | + // the AI block below: the set of sitemaps an install publishes is a | |
| 967 | + // runtime fact. `robots_txt_content` is a snapshot of it taken at the | |
| 968 | + // last save, and nothing invalidated that snapshot, so deactivating a | |
| 969 | + // sitemap provider left its URL advertised and serving HTML (#835). | |
| 970 | + // Composing it on every render means the advertisement agrees with what | |
| 971 | + // the install publishes, in both directions, with no cache to expire. | |
| 718 | 972 | if (!$fully_blocked) { |
| 973 | + $body = $this->apply_sitemap_block($body); | |
| 974 | + } | |
| 975 | + | |
| 976 | + if (!$fully_blocked) { | |
| 719 | 977 | $body = $this->apply_ai_crawler_block($body, $settings); |
| 720 | 978 | } |
| 721 | 979 | |
| 722 | 980 | if ($body === '') { |
| @@ -726,8 +984,87 @@ | ||
| 726 | 984 | return $this->robots_txt_header() . $body . "\n"; |
| 727 | 985 | } |
| 728 | 986 | |
| 729 | 987 | /** |
| 988 | + * Replace the generated Sitemap block with the one this install publishes. | |
| 989 | + * | |
| 990 | + * @since 2.14.0 | |
| 991 | + * @param string $body Robots.txt body, without the header. | |
| 992 | + * @return string | |
| 993 | + */ | |
| 994 | + private function apply_sitemap_block(string $body): string { | |
| 995 | + $stripped = $this->strip_generated_sitemap_block($body); | |
| 996 | + $urls = $this->get_sitemap_urls_for_robots(); | |
| 997 | + | |
| 998 | + if (empty($urls)) { | |
| 999 | + return $stripped; | |
| 1000 | + } | |
| 1001 | + | |
| 1002 | + $block = ''; | |
| 1003 | + foreach ($urls as $url) { | |
| 1004 | + $block .= 'Sitemap: ' . $url . "\n"; | |
| 1005 | + } | |
| 1006 | + | |
| 1007 | + if ('' === trim($stripped)) { | |
| 1008 | + return trim($block); | |
| 1009 | + } | |
| 1010 | + | |
| 1011 | + // The grammar build_robots_txt_content() writes: one blank line before | |
| 1012 | + // the block, none inside it. A blank line terminates a record in the | |
| 1013 | + // robots.txt grammar, so a line between every directive is invalid. | |
| 1014 | + return rtrim($stripped) . "\n\n" . trim($block); | |
| 1015 | + } | |
| 1016 | + | |
| 1017 | + /** | |
| 1018 | + * Remove the plugin-written Sitemap block from a stored body. | |
| 1019 | + * | |
| 1020 | + * Only the trailing run of `Sitemap:` lines is removed, which is the exact | |
| 1021 | + * shape `build_robots_txt_content()` writes: a blank line, then nothing but | |
| 1022 | + * `Sitemap:` lines to the end of the body. A `Sitemap:` line anywhere else | |
| 1023 | + * was typed by the site owner and is left exactly where they put it, which | |
| 1024 | + * is why this cannot simply strip every matching line. | |
| 1025 | + * | |
| 1026 | + * @since 2.14.0 | |
| 1027 | + * @param string $body Robots.txt body. | |
| 1028 | + * @return string | |
| 1029 | + */ | |
| 1030 | + private function strip_generated_sitemap_block(string $body): string { | |
| 1031 | + $lines = preg_split('/\R/', $body); | |
| 1032 | + | |
| 1033 | + if (!is_array($lines)) { | |
| 1034 | + return $body; | |
| 1035 | + } | |
| 1036 | + | |
| 1037 | + $cut = count($lines); | |
| 1038 | + | |
| 1039 | + // Walk back over the trailing block: sitemap lines, and the blank lines | |
| 1040 | + // that separate or pad it. Anything else ends the block. | |
| 1041 | + for ($i = count($lines) - 1; $i >= 0; $i--) { | |
| 1042 | + $line = trim($lines[$i]); | |
| 1043 | + | |
| 1044 | + if ('' === $line) { | |
| 1045 | + $cut = $i; | |
| 1046 | + continue; | |
| 1047 | + } | |
| 1048 | + | |
| 1049 | + if (0 === stripos($line, 'sitemap:')) { | |
| 1050 | + $cut = $i; | |
| 1051 | + continue; | |
| 1052 | + } | |
| 1053 | + | |
| 1054 | + break; | |
| 1055 | + } | |
| 1056 | + | |
| 1057 | + if ($cut >= count($lines)) { | |
| 1058 | + return $body; | |
| 1059 | + } | |
| 1060 | + | |
| 1061 | + // Nothing but sitemap lines in the whole body means there is no owner | |
| 1062 | + // content to keep. | |
| 1063 | + return rtrim(implode("\n", array_slice($lines, 0, $cut))); | |
| 1064 | + } | |
| 1065 | + | |
| 1066 | + /** | |
| 730 | 1067 | * Resolve the robots.txt actually served to crawlers, with its origin. |
| 731 | 1068 | * |
| 732 | 1069 | * Lets an API/MCP consumer see the effective output without crawling the |
| 733 | 1070 | * URL. Mirrors serving precedence: a physical robots.txt in the web root is |
| @@ -1178,17 +1515,19 @@ | ||
| 1178 | 1515 | $home_text = $settings['breadcrumb_home_text'] ?? 'Home'; |
| 1179 | 1516 | if (empty($home_text)) { |
| 1180 | 1517 | $optimization['warnings'][] = 'Empty home text reduces accessibility for screen readers'; |
| 1181 | 1518 | $optimization['score'] -= 15; |
| 1182 | - } elseif (strlen($home_text) > 20) { | |
| 1183 | - $optimization['suggestions'][] = 'Keep home text concise (current: ' . strlen($home_text) . ' chars)'; | |
| 1519 | + } elseif (mb_strlen($home_text) > 20) { | |
| 1520 | + // mb_strlen: this number is shown to the user as "chars" (#687). | |
| 1521 | + $optimization['suggestions'][] = 'Keep home text concise (current: ' . mb_strlen($home_text) . ' chars)'; | |
| 1184 | 1522 | $optimization['score'] -= 5; |
| 1185 | 1523 | } |
| 1186 | 1524 | |
| 1187 | 1525 | // Check prefix usage |
| 1188 | 1526 | $prefix = $settings['breadcrumb_prefix'] ?? ''; |
| 1189 | - if (!empty($prefix) && strlen($prefix) > 50) { | |
| 1190 | - $optimization['suggestions'][] = 'Breadcrumb prefix is quite long (' . strlen($prefix) . ' chars) - consider shortening'; | |
| 1527 | + if (!empty($prefix) && mb_strlen($prefix) > 50) { | |
| 1528 | + // mb_strlen: this number is shown to the user as "chars" (#687). | |
| 1529 | + $optimization['suggestions'][] = 'Breadcrumb prefix is quite long (' . mb_strlen($prefix) . ' chars) - consider shortening'; | |
| 1191 | 1530 | $optimization['score'] -= 5; |
| 1192 | 1531 | } |
| 1193 | 1532 | |
| 1194 | 1533 | // Current page display |
| @@ -1319,13 +1658,16 @@ | ||
| 1319 | 1658 | } |
| 1320 | 1659 | |
| 1321 | 1660 | // Additional logo analysis for local images |
| 1322 | 1661 | if (!empty($logo_url) && filter_var($logo_url, FILTER_VALIDATE_URL)) { |
| 1323 | - $attachment_id = attachment_url_to_postid($logo_url); | |
| 1662 | + $attachment_id = Attachment_Lookup::id_from_url($logo_url); | |
| 1324 | 1663 | if ($attachment_id) { |
| 1325 | 1664 | $image_meta = wp_get_attachment_metadata($attachment_id); |
| 1326 | - $width = isset($image_meta['width']) ? (int) $image_meta['width'] : 0; | |
| 1327 | - $height = isset($image_meta['height']) ? (int) $image_meta['height'] : 0; | |
| 1665 | + // The configured file's own size — a logo picked at a generated | |
| 1666 | + // size is not as large as the upload behind it. | |
| 1667 | + $logo_file = Attachment_Lookup::describe($attachment_id, $logo_url); | |
| 1668 | + $width = $logo_file['width']; | |
| 1669 | + $height = $logo_file['height']; | |
| 1328 | 1670 | |
| 1329 | 1671 | // SVG logos store 0x0 metadata — no dimension/ratio analysis |
| 1330 | 1672 | // is possible (and dividing by 0 is fatal). |
| 1331 | 1673 | if ($image_meta && $width > 0 && $height > 0) { |
| @@ -1428,11 +1770,12 @@ | ||
| 1428 | 1770 | $optimization['score'] -= 15; |
| 1429 | 1771 | } |
| 1430 | 1772 | } |
| 1431 | 1773 | |
| 1432 | - // Business type validation | |
| 1433 | - if (empty($settings['business_type'])) { | |
| 1434 | - $optimization['suggestions'][] = 'Select a specific business type for better schema markup'; | |
| 1774 | + // Business type validation (shared rule, one message — #622). | |
| 1775 | + $business_type = $this->business_type_status($settings); | |
| 1776 | + if ('suggestion' === $business_type['status']) { | |
| 1777 | + $optimization['suggestions'][] = $business_type['message']; | |
| 1435 | 1778 | $optimization['score'] -= 5; |
| 1436 | 1779 | } |
| 1437 | 1780 | |
| 1438 | 1781 | // Email validation |
| @@ -1962,24 +2305,17 @@ | ||
| 1962 | 2305 | 'icon' => '✗' |
| 1963 | 2306 | ]; |
| 1964 | 2307 | } |
| 1965 | 2308 | |
| 1966 | - // Business Type validation | |
| 1967 | - if (!empty($settings['business_type']) && $settings['business_type'] !== 'LocalBusiness') { | |
| 1968 | - $field_details[] = [ | |
| 1969 | - 'field' => 'business_type', | |
| 1970 | - 'label' => 'Business type is selected for proper schema markup.', | |
| 1971 | - 'status' => 'valid', | |
| 1972 | - 'icon' => '✓' | |
| 1973 | - ]; | |
| 1974 | - } else { | |
| 1975 | - $field_details[] = [ | |
| 1976 | - 'field' => 'business_type', | |
| 1977 | - 'label' => 'Specific business type selection recommended for better schema markup.', | |
| 1978 | - 'status' => 'suggestion', | |
| 1979 | - 'icon' => '⚠' | |
| 1980 | - ]; | |
| 1981 | - } | |
| 2309 | + // Business Type validation — see business_type_status() for why there | |
| 2310 | + // is exactly one rule here now (#622). | |
| 2311 | + $business_type = $this->business_type_status($settings); | |
| 2312 | + $field_details[] = [ | |
| 2313 | + 'field' => 'business_type', | |
| 2314 | + 'label' => $business_type['message'], | |
| 2315 | + 'status' => $business_type['status'], | |
| 2316 | + 'icon' => 'valid' === $business_type['status'] ? '✓' : '⚠', | |
| 2317 | + ]; | |
| 1982 | 2318 | |
| 1983 | 2319 | // Address validation (NAP consistency) |
| 1984 | 2320 | $address_fields = ['business_address', 'business_city', 'business_state', 'business_country']; |
| 1985 | 2321 | $address_complete = true; |
| @@ -2663,11 +2999,15 @@ | ||
| 2663 | 2999 | } else { |
| 2664 | 3000 | $validation['suggestions'][] = 'Add business hours to improve local search visibility'; |
| 2665 | 3001 | } |
| 2666 | 3002 | |
| 2667 | - // Validate business type | |
| 2668 | - if (empty($settings['business_type'])) { | |
| 2669 | - $validation['suggestions'][] = 'Select a specific business type for better schema markup'; | |
| 3003 | + // Business type, through the shared rule (#622). This is the only place | |
| 3004 | + // it is reported on the generic path: validate_settings() with no tab | |
| 3005 | + // context attaches basic-info field details, not business-info ones, so | |
| 3006 | + // without this the setting would go unreported there entirely. | |
| 3007 | + $business_type = $this->business_type_status($settings); | |
| 3008 | + if ('suggestion' === $business_type['status']) { | |
| 3009 | + $validation['suggestions'][] = $business_type['message']; | |
| 2670 | 3010 | } |
| 2671 | 3011 | |
| 2672 | 3012 | return $validation; |
| 2673 | 3013 | } |
| @@ -2759,13 +3099,54 @@ | ||
| 2759 | 3099 | * @since 2.0.1 |
| 2760 | 3100 | * |
| 2761 | 3101 | * @return string[] |
| 2762 | 3102 | */ |
| 3103 | + /** | |
| 3104 | + * The stored alternate name(s), shaped for schema output. | |
| 3105 | + * | |
| 3106 | + * schema.org and Google both allow `alternateName` to carry one value or | |
| 3107 | + * several, and the store already round-trips either shape, so this accepts | |
| 3108 | + * both and normalises: null when there is nothing to publish, a bare string | |
| 3109 | + * for one name, a list for more. Emitting a one-element array would be | |
| 3110 | + * valid but noisier than it needs to be. | |
| 3111 | + * | |
| 3112 | + * Shared because both WebSite producers need it and must agree — a property | |
| 3113 | + * added to one and not the other is how #688 happened. | |
| 3114 | + * | |
| 3115 | + * @since 2.7.0 | |
| 3116 | + * | |
| 3117 | + * @param mixed $value Stored alternate_name value. | |
| 3118 | + * @return string|string[]|null | |
| 3119 | + */ | |
| 3120 | + public static function alternate_name_for_schema($value) { | |
| 3121 | + $names = []; | |
| 3122 | + | |
| 3123 | + foreach ((array) $value as $name) { | |
| 3124 | + if (!is_scalar($name)) { | |
| 3125 | + continue; | |
| 3126 | + } | |
| 3127 | + | |
| 3128 | + $name = trim((string) $name); | |
| 3129 | + | |
| 3130 | + if ('' !== $name && !in_array($name, $names, true)) { | |
| 3131 | + $names[] = $name; | |
| 3132 | + } | |
| 3133 | + } | |
| 3134 | + | |
| 3135 | + if (empty($names)) { | |
| 3136 | + return null; | |
| 3137 | + } | |
| 3138 | + | |
| 3139 | + return 1 === count($names) ? $names[0] : $names; | |
| 3140 | + } | |
| 3141 | + | |
| 2763 | 3142 | protected function additional_setting_keys(): array { |
| 2764 | 3143 | return [ |
| 2765 | 3144 | // Title formats, one per context. |
| 2766 | 3145 | 'homepage_title', 'post_title', 'page_title', 'category_title', |
| 2767 | 3146 | 'tag_title', 'author_title', 'search_title', 'archive_title', |
| 3147 | + // The blog-index homepage's meta description (#897). | |
| 3148 | + 'homepage_description', | |
| 2768 | 3149 | // Breadcrumbs. |
| 2769 | 3150 | 'breadcrumb_prefix', 'show_current_page', 'breadcrumb_use_seo_title', |
| 2770 | 3151 | // Identity, as written by the setup wizard and the importers. |
| 2771 | 3152 | 'alternate_name', 'identity_type', 'represents', |
| @@ -2811,12 +3192,96 @@ | ||
| 2811 | 3192 | if (array_key_exists('ai_crawler_rules', $sanitized)) { |
| 2812 | 3193 | $sanitized['ai_crawler_rules'] = AI_Crawlers::normalize_rules($sanitized['ai_crawler_rules']); |
| 2813 | 3194 | } |
| 2814 | 3195 | |
| 3196 | + // Same reasoning one key up, for the scheme override (#638). Anything | |
| 3197 | + // that is not one of the three modes means "follow WordPress", and is | |
| 3198 | + // stored as that rather than kept verbatim — otherwise get-site-identity | |
| 3199 | + // -settings would report a scheme the site does not actually publish. | |
| 3200 | + if (array_key_exists('canonical_scheme', $sanitized)) { | |
| 3201 | + $sanitized['canonical_scheme'] = in_array($sanitized['canonical_scheme'], Url_Scheme::MODES, true) | |
| 3202 | + ? $sanitized['canonical_scheme'] | |
| 3203 | + : Url_Scheme::AUTOMATIC; | |
| 3204 | + } | |
| 3205 | + | |
| 3206 | + // Same reasoning again for the business type. It goes straight into | |
| 3207 | + // LocalBusiness schema, so a type that is not in the schema.org | |
| 3208 | + // vocabulary is invalid structured data — and storing it verbatim would | |
| 3209 | + // have get-site-identity-settings report a type the site cannot | |
| 3210 | + // actually publish. An empty value keeps meaning "not set"; anything | |
| 3211 | + // else unrecognised falls back to the general-purpose root (#623). | |
| 3212 | + if (array_key_exists('business_type', $sanitized)) { | |
| 3213 | + $type = (string) $sanitized['business_type']; | |
| 3214 | + | |
| 3215 | + if ('' !== $type && !\ThinkRank\Config\Local_Business_Types_Config::is_valid($type)) { | |
| 3216 | + $type = \ThinkRank\Config\Local_Business_Types_Config::ROOT; | |
| 3217 | + } | |
| 3218 | + | |
| 3219 | + $sanitized['business_type'] = $type; | |
| 3220 | + } | |
| 3221 | + | |
| 2815 | 3222 | return $sanitized; |
| 2816 | 3223 | } |
| 2817 | 3224 | |
| 2818 | 3225 | /** |
| 3226 | + * schema.org's general-purpose LocalBusiness type. | |
| 3227 | + * | |
| 3228 | + * The default, the first option in the control, and a valid answer in its | |
| 3229 | + * own right — which is the whole point of #622. | |
| 3230 | + * | |
| 3231 | + * @since 2.10.0 | |
| 3232 | + * @var string | |
| 3233 | + */ | |
| 3234 | + private const GENERAL_BUSINESS_TYPE = 'LocalBusiness'; | |
| 3235 | + | |
| 3236 | + /** | |
| 3237 | + * The one rule for whether a business type needs the user's attention. | |
| 3238 | + * | |
| 3239 | + * There were three, with two wordings and two different conditions. Two | |
| 3240 | + * fired when the value was empty; the third fired when it WAS | |
| 3241 | + * `LocalBusiness` — which is the default, the first option in the control | |
| 3242 | + * and a perfectly valid schema.org type. So the warning appeared out of the | |
| 3243 | + * box for every site, could not be cleared without choosing a type that | |
| 3244 | + * might be inaccurate, and on an empty value it appeared three times in two | |
| 3245 | + * different phrasings, which is why it was reported as showing twice (#622). | |
| 3246 | + * | |
| 3247 | + * The rule now: a type is expected, and any type in the vocabulary is a | |
| 3248 | + * correct answer. Only an unset value is worth prompting about. | |
| 3249 | + * `LocalBusiness` is the general-purpose answer and is accepted as one — | |
| 3250 | + * with a note that a more specific type sharpens the schema, phrased as the | |
| 3251 | + * guidance it is rather than as a fault the user has to clear. | |
| 3252 | + * | |
| 3253 | + * @since 2.10.0 | |
| 3254 | + * | |
| 3255 | + * @param array $settings Site identity settings. | |
| 3256 | + * @return array{status:string,message:string} `valid` or `suggestion`. | |
| 3257 | + */ | |
| 3258 | + private function business_type_status(array $settings): array { | |
| 3259 | + $type = trim((string) ($settings['business_type'] ?? '')); | |
| 3260 | + | |
| 3261 | + if ('' === $type) { | |
| 3262 | + return [ | |
| 3263 | + 'status' => 'suggestion', | |
| 3264 | + 'message' => __('Select a business type so your local schema describes the right kind of business.', 'thinkrank'), | |
| 3265 | + ]; | |
| 3266 | + } | |
| 3267 | + | |
| 3268 | + // The literal rather than a constant from the expanded type list (#623): | |
| 3269 | + // that lands on its own branch, and this fix must not wait on it. | |
| 3270 | + if (self::GENERAL_BUSINESS_TYPE === $type) { | |
| 3271 | + return [ | |
| 3272 | + 'status' => 'valid', | |
| 3273 | + 'message' => __('Business type is set to Local Business. A more specific type sharpens your schema, if one fits.', 'thinkrank'), | |
| 3274 | + ]; | |
| 3275 | + } | |
| 3276 | + | |
| 3277 | + return [ | |
| 3278 | + 'status' => 'valid', | |
| 3279 | + 'message' => __('Business type is selected for proper schema markup.', 'thinkrank'), | |
| 3280 | + ]; | |
| 3281 | + } | |
| 3282 | + | |
| 3283 | + /** | |
| 2819 | 3284 | * Get default settings for a context type (implements interface) |
| 2820 | 3285 | * |
| 2821 | 3286 | * @since 1.0.0 |
| 2822 | 3287 | * |
| @@ -2836,8 +3301,27 @@ | ||
| 2836 | 3301 | 'breadcrumb_home_text' => 'Home', |
| 2837 | 3302 | 'breadcrumb_separator' => '>', |
| 2838 | 3303 | 'robots_txt_enabled' => true, |
| 2839 | 3304 | 'allow_search_engines' => true, |
| 3305 | + // Answer 404 when a content selector in the URL resolved to | |
| 3306 | + // nothing (#634). On by default, unlike the other new settings | |
| 3307 | + // here: it changes no URL a visitor or a correct crawler uses, only | |
| 3308 | + // ones where WordPress resolved nothing and served the blog listing | |
| 3309 | + // at 200 anyway. | |
| 3310 | + 'query_protection' => true, | |
| 3311 | + | |
| 3312 | + // Feed controls (#635). All three off, so an upgrade changes | |
| 3313 | + // nothing about what an existing site already sends its | |
| 3314 | + // subscribers; a brand-new install is seeded with the signature and | |
| 3315 | + // the noindex on, in Activator::seed_feed_defaults(). | |
| 3316 | + 'feed_excerpt_only' => false, | |
| 3317 | + 'feed_source_link' => false, | |
| 3318 | + 'feed_noindex' => false, | |
| 3319 | + | |
| 3320 | + // The scheme self-referential URLs go out with (#638). 'automatic' | |
| 3321 | + // means substitute nothing and follow WordPress, which is what | |
| 3322 | + // every site did before the setting existed. | |
| 3323 | + 'canonical_scheme' => Url_Scheme::AUTOMATIC, | |
| 2840 | 3324 | 'robots_txt_content' => '', |
| 2841 | 3325 | // Empty map = every AI crawler allowed. Defaults must stay |
| 2842 | 3326 | // permissive so an upgrade never starts blocking a crawler a site |
| 2843 | 3327 | // was happily serving (#657). |
| @@ -3051,16 +3535,17 @@ | ||
| 3051 | 3535 | // Remove extra whitespace |
| 3052 | 3536 | $title = preg_replace('/\s+/', ' ', $title); |
| 3053 | 3537 | $title = trim($title); |
| 3054 | 3538 | |
| 3055 | - // Ensure title is not too long (60 characters max for SEO) | |
| 3056 | - if (strlen($title) > 60) { | |
| 3057 | - // Try to truncate at word boundary | |
| 3058 | - $title = wp_trim_words($title, 8, '...'); | |
| 3059 | - if (strlen($title) > 60) { | |
| 3060 | - $title = substr($title, 0, 57) . '...'; | |
| 3061 | - } | |
| 3062 | - } | |
| 3539 | + // Ensure title is not too long (60 characters max for SEO). | |
| 3540 | + // All three units here were wrong for non-Latin text: strlen() counts | |
| 3541 | + // BYTES so the gate fired at 20 Thai characters, wp_trim_words() counts | |
| 3542 | + // CHARACTERS on th/ja/zh_* so `8` cut the title to 8 of them, and | |
| 3543 | + // substr() cuts bytes so it split a character mid-sequence (#687). | |
| 3544 | + $title = \ThinkRank\Core\Seo_Text::trim_to_length( | |
| 3545 | + $title, | |
| 3546 | + \ThinkRank\Core\Seo_Text::TITLE_MAX_LENGTH | |
| 3547 | + ); | |
| 3063 | 3548 | |
| 3064 | 3549 | // Ensure title is not empty |
| 3065 | 3550 | if (empty($title)) { |
| 3066 | 3551 | $title = get_bloginfo('name'); |
| @@ -3450,8 +3935,29 @@ | ||
| 3450 | 3935 | * @since 1.0.0 |
| 3451 | 3936 | * @return array Array of sitemap URLs |
| 3452 | 3937 | */ |
| 3453 | 3938 | private function get_sitemap_urls_for_robots(): array { |
| 3939 | + // One wrapper over every return path below, including the #104 extras. | |
| 3940 | + // The Sitemap: line is the only absolute URL of ours in robots.txt and | |
| 3941 | + // the one a crawler follows to find everything else, so it has to carry | |
| 3942 | + // the site's scheme preference (#638). Applied here rather than where | |
| 3943 | + // the body is assembled, because that path also renders a robots.txt a | |
| 3944 | + // site owner typed themselves, and their text is not ours to rewrite. | |
| 3945 | + return array_map( | |
| 3946 | + static function (string $url): string { | |
| 3947 | + return Url_Scheme::apply($url); | |
| 3948 | + }, | |
| 3949 | + $this->collect_sitemap_urls_for_robots() | |
| 3950 | + ); | |
| 3951 | + } | |
| 3952 | + | |
| 3953 | + /** | |
| 3954 | + * The sitemap URLs robots.txt advertises, before the scheme preference. | |
| 3955 | + * | |
| 3956 | + * @since 1.0.0 | |
| 3957 | + * @return array Array of sitemap URLs | |
| 3958 | + */ | |
| 3959 | + private function collect_sitemap_urls_for_robots(): array { | |
| 3454 | 3960 | try { |
| 3455 | 3961 | // Get sitemap settings |
| 3456 | 3962 | $sitemap_generator = new \ThinkRank\SEO\Sitemap_Generator(); |
| 3457 | 3963 | $sitemap_settings = $sitemap_generator->get_settings('site'); |
| @@ -3484,11 +3990,29 @@ | ||
| 3484 | 3990 | } |
| 3485 | 3991 | } |
| 3486 | 3992 | |
| 3487 | 3993 | if ($index_url !== '') { |
| 3488 | - // The index alone — it covers the children and, on a segmented | |
| 3489 | - // install, the local business sitemap too. | |
| 3490 | - return [$index_url]; | |
| 3994 | + // The index covers the children and, on a segmented install, | |
| 3995 | + // the local business sitemap too. | |
| 3996 | + // | |
| 3997 | + // It does not cover a sitemap contributed through | |
| 3998 | + // `thinkrank_additional_sitemaps`: the index is built by this | |
| 3999 | + // plugin's own generator and never lists them. Returning the | |
| 4000 | + // index alone therefore left a contributed sitemap with no | |
| 4001 | + // discovery path at all — absent from robots.txt and absent | |
| 4002 | + // from the index — so Pro's News sitemap was unreachable on any | |
| 4003 | + // install with the index enabled, which is the default (#835). | |
| 4004 | + $contributed = []; | |
| 4005 | + | |
| 4006 | + foreach (\ThinkRank\SEO\Sitemap_Generator::additional_sitemaps() as $path) { | |
| 4007 | + $url = home_url($path); | |
| 4008 | + | |
| 4009 | + if ($url !== $index_url && !in_array($url, $contributed, true)) { | |
| 4010 | + $contributed[] = $url; | |
| 4011 | + } | |
| 4012 | + } | |
| 4013 | + | |
| 4014 | + return array_merge([$index_url], $contributed); | |
| 3491 | 4015 | } |
| 3492 | 4016 | |
| 3493 | 4017 | // Fallback to default if no URLs found |
| 3494 | 4018 | if (empty($sitemap_urls)) { |
| @@ -3500,9 +4024,22 @@ | ||
| 3500 | 4024 | // business sitemap and the sitemaps other plugins register both land |
| 3501 | 4025 | // here for the same reason, so they go through one list (#104). |
| 3502 | 4026 | $extra = []; |
| 3503 | 4027 | |
| 3504 | - if (file_exists(ABSPATH . 'local-sitemap.xml')) { | |
| 4028 | + // Not a file test. Under dynamic delivery the local sitemap is | |
| 4029 | + // served from PHP and no file is ever written, so file_exists() | |
| 4030 | + // silently dropped a sitemap the site really does publish (#752). | |
| 4031 | + // On static sites the file is still what proves it, so both count. | |
| 4032 | + $local_sitemap_published = file_exists(ABSPATH . 'local-sitemap.xml'); | |
| 4033 | + | |
| 4034 | + if (!$local_sitemap_published && class_exists('ThinkRank\\SEO\\Sitemap_Generator')) { | |
| 4035 | + $generator = new \ThinkRank\SEO\Sitemap_Generator(false); | |
| 4036 | + | |
| 4037 | + $local_sitemap_published = 'dynamic' === $generator->resolve_delivery_mode() | |
| 4038 | + && $generator->publishes_local_sitemap(); | |
| 4039 | + } | |
| 4040 | + | |
| 4041 | + if ($local_sitemap_published) { | |
| 3505 | 4042 | $extra[] = '/local-sitemap.xml'; |
| 3506 | 4043 | } |
| 3507 | 4044 | |
| 3508 | 4045 | foreach (\ThinkRank\SEO\Sitemap_Generator::additional_sitemaps() as $path) { |
| @@ -3927,11 +4464,14 @@ | ||
| 3927 | 4464 | $optimization['validation']['valid'] = false; |
| 3928 | 4465 | } |
| 3929 | 4466 | |
| 3930 | 4467 | if (!empty($value) && isset($config['max_length'])) { |
| 3931 | - if (strlen($value) > $config['max_length']) { | |
| 4468 | + // The warning says "characters", so measure and cut in characters: | |
| 4469 | + // strlen()/substr() fired early on non-Latin values and the | |
| 4470 | + // suggested replacement was cut mid-character (#687). | |
| 4471 | + if (mb_strlen($value) > $config['max_length']) { | |
| 3932 | 4472 | $optimization['validation']['warnings'][] = "{$element} exceeds maximum length of {$config['max_length']} characters"; |
| 3933 | - $optimization['optimized_value'] = substr($value, 0, $config['max_length']); | |
| 4473 | + $optimization['optimized_value'] = \ThinkRank\Core\Seo_Text::trim_to_length($value, (int) $config['max_length']); | |
| 3934 | 4474 | } |
| 3935 | 4475 | } |
| 3936 | 4476 | |
| 3937 | 4477 | // SEO-specific optimizations |
| @@ -3970,18 +4510,20 @@ | ||
| 3970 | 4510 | return $optimization; |
| 3971 | 4511 | } |
| 3972 | 4512 | |
| 3973 | 4513 | // Check if it's a local image |
| 3974 | - $attachment_id = attachment_url_to_postid($value); | |
| 4514 | + $attachment_id = Attachment_Lookup::id_from_url($value); | |
| 3975 | 4515 | if ($attachment_id) { |
| 3976 | 4516 | $image_meta = wp_get_attachment_metadata($attachment_id); |
| 3977 | 4517 | |
| 3978 | 4518 | if ($image_meta && isset($image_meta['width'], $image_meta['height'])) { |
| 3979 | - // Check recommended size | |
| 4519 | + // Check recommended size, against the configured file itself | |
| 4520 | + // rather than the upload it may have been generated from. | |
| 3980 | 4521 | if (isset($config['recommended_size'])) { |
| 3981 | 4522 | [$rec_width, $rec_height] = explode('x', $config['recommended_size']); |
| 4523 | + $image_file = Attachment_Lookup::describe($attachment_id, $value); | |
| 3982 | 4524 | |
| 3983 | - if ((int) $image_meta['width'] !== (int) $rec_width || (int) $image_meta['height'] !== (int) $rec_height) { | |
| 4525 | + if ($image_file['width'] !== (int) $rec_width || $image_file['height'] !== (int) $rec_height) { | |
| 3984 | 4526 | $optimization['suggestions'][] = "Consider using {$config['recommended_size']} size for optimal {$element}"; |
| 3985 | 4527 | } |
| 3986 | 4528 | } |
| 3987 | 4529 | |