| @@ -508,11 +508,12 @@ | ||
| 508 | 508 | * Attachment ID behind a configured icon URL, or 0 when it is not ours. |
| 509 | 509 | * |
| 510 | 510 | * attachment_url_to_postid() matches _wp_attached_file, which holds the |
| 511 | 511 | * ORIGINAL upload path, so the URL of a generated derivative |
| 512 | - * (`logo-512.png`) returns 0 — and that is exactly what the media picker | |
| 513 | - * hands back when the user chooses a size. Strip the dimension suffix and | |
| 514 | - * try the original once. | |
| 512 | + * (`logo-512x512.png`) returns 0 — and that is exactly what the media | |
| 513 | + * picker hands back when the user chooses a size. Attachment_Lookup falls | |
| 514 | + * back to the original behind it; the fallback started here and moved | |
| 515 | + * there when every other image lookup turned out to need it (#847). | |
| 515 | 516 | * |
| 516 | 517 | * Shared with SEO_Manager's site-icon filter so both sides of the feature |
| 517 | 518 | * agree on which attachment a configured URL means. |
| 518 | 519 | * |
| @@ -521,21 +522,9 @@ | ||
| 521 | 522 | * @param string $url Configured icon URL. |
| 522 | 523 | * @return int Attachment ID, or 0. |
| 523 | 524 | */ |
| 524 | 525 | public static function icon_attachment_id(string $url): int { |
| 525 | - $attachment_id = (int) attachment_url_to_postid($url); | |
| 526 | - | |
| 527 | - if ($attachment_id) { | |
| 528 | - return $attachment_id; | |
| 529 | - } | |
| 530 | - | |
| 531 | - $original = preg_replace('/-\d+x\d+(?=\.[a-zA-Z0-9]+$)/', '', $url); | |
| 532 | - | |
| 533 | - if (is_string($original) && $original !== $url) { | |
| 534 | - return (int) attachment_url_to_postid($original); | |
| 535 | - } | |
| 536 | - | |
| 537 | - return 0; | |
| 526 | + return Attachment_Lookup::id_from_url($url); | |
| 538 | 527 | } |
| 539 | 528 | |
| 540 | 529 | /** |
| 541 | 530 | * Which ICON_SIZES derivatives this attachment still needs. |
| @@ -1432,13 +1421,16 @@ | ||
| 1432 | 1421 | } |
| 1433 | 1422 | |
| 1434 | 1423 | // Additional logo analysis for local images |
| 1435 | 1424 | if (!empty($logo_url) && filter_var($logo_url, FILTER_VALIDATE_URL)) { |
| 1436 | - $attachment_id = attachment_url_to_postid($logo_url); | |
| 1425 | + $attachment_id = Attachment_Lookup::id_from_url($logo_url); | |
| 1437 | 1426 | if ($attachment_id) { |
| 1438 | 1427 | $image_meta = wp_get_attachment_metadata($attachment_id); |
| 1439 | - $width = isset($image_meta['width']) ? (int) $image_meta['width'] : 0; | |
| 1440 | - $height = isset($image_meta['height']) ? (int) $image_meta['height'] : 0; | |
| 1428 | + // The configured file's own size — a logo picked at a generated | |
| 1429 | + // size is not as large as the upload behind it. | |
| 1430 | + $logo_file = Attachment_Lookup::describe($attachment_id, $logo_url); | |
| 1431 | + $width = $logo_file['width']; | |
| 1432 | + $height = $logo_file['height']; | |
| 1441 | 1433 | |
| 1442 | 1434 | // SVG logos store 0x0 metadata — no dimension/ratio analysis |
| 1443 | 1435 | // is possible (and dividing by 0 is fatal). |
| 1444 | 1436 | if ($image_meta && $width > 0 && $height > 0) { |
| @@ -1541,11 +1533,12 @@ | ||
| 1541 | 1533 | $optimization['score'] -= 15; |
| 1542 | 1534 | } |
| 1543 | 1535 | } |
| 1544 | 1536 | |
| 1545 | - // Business type validation | |
| 1546 | - if (empty($settings['business_type'])) { | |
| 1547 | - $optimization['suggestions'][] = 'Select a specific business type for better schema markup'; | |
| 1537 | + // Business type validation (shared rule, one message — #622). | |
| 1538 | + $business_type = $this->business_type_status($settings); | |
| 1539 | + if ('suggestion' === $business_type['status']) { | |
| 1540 | + $optimization['suggestions'][] = $business_type['message']; | |
| 1548 | 1541 | $optimization['score'] -= 5; |
| 1549 | 1542 | } |
| 1550 | 1543 | |
| 1551 | 1544 | // Email validation |
| @@ -2075,24 +2068,17 @@ | ||
| 2075 | 2068 | 'icon' => '✗' |
| 2076 | 2069 | ]; |
| 2077 | 2070 | } |
| 2078 | 2071 | |
| 2079 | - // Business Type validation | |
| 2080 | - if (!empty($settings['business_type']) && $settings['business_type'] !== 'LocalBusiness') { | |
| 2081 | - $field_details[] = [ | |
| 2082 | - 'field' => 'business_type', | |
| 2083 | - 'label' => 'Business type is selected for proper schema markup.', | |
| 2084 | - 'status' => 'valid', | |
| 2085 | - 'icon' => '✓' | |
| 2086 | - ]; | |
| 2087 | - } else { | |
| 2088 | - $field_details[] = [ | |
| 2089 | - 'field' => 'business_type', | |
| 2090 | - 'label' => 'Specific business type selection recommended for better schema markup.', | |
| 2091 | - 'status' => 'suggestion', | |
| 2092 | - 'icon' => '⚠' | |
| 2093 | - ]; | |
| 2094 | - } | |
| 2072 | + // Business Type validation — see business_type_status() for why there | |
| 2073 | + // is exactly one rule here now (#622). | |
| 2074 | + $business_type = $this->business_type_status($settings); | |
| 2075 | + $field_details[] = [ | |
| 2076 | + 'field' => 'business_type', | |
| 2077 | + 'label' => $business_type['message'], | |
| 2078 | + 'status' => $business_type['status'], | |
| 2079 | + 'icon' => 'valid' === $business_type['status'] ? '✓' : '⚠', | |
| 2080 | + ]; | |
| 2095 | 2081 | |
| 2096 | 2082 | // Address validation (NAP consistency) |
| 2097 | 2083 | $address_fields = ['business_address', 'business_city', 'business_state', 'business_country']; |
| 2098 | 2084 | $address_complete = true; |
| @@ -2776,11 +2762,15 @@ | ||
| 2776 | 2762 | } else { |
| 2777 | 2763 | $validation['suggestions'][] = 'Add business hours to improve local search visibility'; |
| 2778 | 2764 | } |
| 2779 | 2765 | |
| 2780 | - // Validate business type | |
| 2781 | - if (empty($settings['business_type'])) { | |
| 2782 | - $validation['suggestions'][] = 'Select a specific business type for better schema markup'; | |
| 2766 | + // Business type, through the shared rule (#622). This is the only place | |
| 2767 | + // it is reported on the generic path: validate_settings() with no tab | |
| 2768 | + // context attaches basic-info field details, not business-info ones, so | |
| 2769 | + // without this the setting would go unreported there entirely. | |
| 2770 | + $business_type = $this->business_type_status($settings); | |
| 2771 | + if ('suggestion' === $business_type['status']) { | |
| 2772 | + $validation['suggestions'][] = $business_type['message']; | |
| 2783 | 2773 | } |
| 2784 | 2774 | |
| 2785 | 2775 | return $validation; |
| 2786 | 2776 | } |
| @@ -2973,12 +2963,86 @@ | ||
| 2973 | 2963 | ? $sanitized['canonical_scheme'] |
| 2974 | 2964 | : Url_Scheme::AUTOMATIC; |
| 2975 | 2965 | } |
| 2976 | 2966 | |
| 2967 | + // Same reasoning again for the business type. It goes straight into | |
| 2968 | + // LocalBusiness schema, so a type that is not in the schema.org | |
| 2969 | + // vocabulary is invalid structured data — and storing it verbatim would | |
| 2970 | + // have get-site-identity-settings report a type the site cannot | |
| 2971 | + // actually publish. An empty value keeps meaning "not set"; anything | |
| 2972 | + // else unrecognised falls back to the general-purpose root (#623). | |
| 2973 | + if (array_key_exists('business_type', $sanitized)) { | |
| 2974 | + $type = (string) $sanitized['business_type']; | |
| 2975 | + | |
| 2976 | + if ('' !== $type && !\ThinkRank\Config\Local_Business_Types_Config::is_valid($type)) { | |
| 2977 | + $type = \ThinkRank\Config\Local_Business_Types_Config::ROOT; | |
| 2978 | + } | |
| 2979 | + | |
| 2980 | + $sanitized['business_type'] = $type; | |
| 2981 | + } | |
| 2982 | + | |
| 2977 | 2983 | return $sanitized; |
| 2978 | 2984 | } |
| 2979 | 2985 | |
| 2980 | 2986 | /** |
| 2987 | + * schema.org's general-purpose LocalBusiness type. | |
| 2988 | + * | |
| 2989 | + * The default, the first option in the control, and a valid answer in its | |
| 2990 | + * own right — which is the whole point of #622. | |
| 2991 | + * | |
| 2992 | + * @since 2.10.0 | |
| 2993 | + * @var string | |
| 2994 | + */ | |
| 2995 | + private const GENERAL_BUSINESS_TYPE = 'LocalBusiness'; | |
| 2996 | + | |
| 2997 | + /** | |
| 2998 | + * The one rule for whether a business type needs the user's attention. | |
| 2999 | + * | |
| 3000 | + * There were three, with two wordings and two different conditions. Two | |
| 3001 | + * fired when the value was empty; the third fired when it WAS | |
| 3002 | + * `LocalBusiness` — which is the default, the first option in the control | |
| 3003 | + * and a perfectly valid schema.org type. So the warning appeared out of the | |
| 3004 | + * box for every site, could not be cleared without choosing a type that | |
| 3005 | + * might be inaccurate, and on an empty value it appeared three times in two | |
| 3006 | + * different phrasings, which is why it was reported as showing twice (#622). | |
| 3007 | + * | |
| 3008 | + * The rule now: a type is expected, and any type in the vocabulary is a | |
| 3009 | + * correct answer. Only an unset value is worth prompting about. | |
| 3010 | + * `LocalBusiness` is the general-purpose answer and is accepted as one — | |
| 3011 | + * with a note that a more specific type sharpens the schema, phrased as the | |
| 3012 | + * guidance it is rather than as a fault the user has to clear. | |
| 3013 | + * | |
| 3014 | + * @since 2.10.0 | |
| 3015 | + * | |
| 3016 | + * @param array $settings Site identity settings. | |
| 3017 | + * @return array{status:string,message:string} `valid` or `suggestion`. | |
| 3018 | + */ | |
| 3019 | + private function business_type_status(array $settings): array { | |
| 3020 | + $type = trim((string) ($settings['business_type'] ?? '')); | |
| 3021 | + | |
| 3022 | + if ('' === $type) { | |
| 3023 | + return [ | |
| 3024 | + 'status' => 'suggestion', | |
| 3025 | + 'message' => __('Select a business type so your local schema describes the right kind of business.', 'thinkrank'), | |
| 3026 | + ]; | |
| 3027 | + } | |
| 3028 | + | |
| 3029 | + // The literal rather than a constant from the expanded type list (#623): | |
| 3030 | + // that lands on its own branch, and this fix must not wait on it. | |
| 3031 | + if (self::GENERAL_BUSINESS_TYPE === $type) { | |
| 3032 | + return [ | |
| 3033 | + 'status' => 'valid', | |
| 3034 | + 'message' => __('Business type is set to Local Business. A more specific type sharpens your schema, if one fits.', 'thinkrank'), | |
| 3035 | + ]; | |
| 3036 | + } | |
| 3037 | + | |
| 3038 | + return [ | |
| 3039 | + 'status' => 'valid', | |
| 3040 | + 'message' => __('Business type is selected for proper schema markup.', 'thinkrank'), | |
| 3041 | + ]; | |
| 3042 | + } | |
| 3043 | + | |
| 3044 | + /** | |
| 2981 | 3045 | * Get default settings for a context type (implements interface) |
| 2982 | 3046 | * |
| 2983 | 3047 | * @since 1.0.0 |
| 2984 | 3048 | * |
| @@ -4189,18 +4253,20 @@ | ||
| 4189 | 4253 | return $optimization; |
| 4190 | 4254 | } |
| 4191 | 4255 | |
| 4192 | 4256 | // Check if it's a local image |
| 4193 | - $attachment_id = attachment_url_to_postid($value); | |
| 4257 | + $attachment_id = Attachment_Lookup::id_from_url($value); | |
| 4194 | 4258 | if ($attachment_id) { |
| 4195 | 4259 | $image_meta = wp_get_attachment_metadata($attachment_id); |
| 4196 | 4260 | |
| 4197 | 4261 | if ($image_meta && isset($image_meta['width'], $image_meta['height'])) { |
| 4198 | - // Check recommended size | |
| 4262 | + // Check recommended size, against the configured file itself | |
| 4263 | + // rather than the upload it may have been generated from. | |
| 4199 | 4264 | if (isset($config['recommended_size'])) { |
| 4200 | 4265 | [$rec_width, $rec_height] = explode('x', $config['recommended_size']); |
| 4266 | + $image_file = Attachment_Lookup::describe($attachment_id, $value); | |
| 4201 | 4267 | |
| 4202 | - if ((int) $image_meta['width'] !== (int) $rec_width || (int) $image_meta['height'] !== (int) $rec_height) { | |
| 4268 | + if ($image_file['width'] !== (int) $rec_width || $image_file['height'] !== (int) $rec_height) { | |
| 4203 | 4269 | $optimization['suggestions'][] = "Consider using {$config['recommended_size']} size for optimal {$element}"; |
| 4204 | 4270 | } |
| 4205 | 4271 | } |
| 4206 | 4272 | |
