Make WordPress Core

Opened 18 months ago

Closed 18 months ago

Last modified 15 months ago

#58197 closed defect (bug) (invalid)

$blog_id always returns 0 in get_site_icon_url hook

Reported by: revxx14's profile revxx14 Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Customize Keywords:
Focuses: multisite Cc:

Description

I recently tried to set up a default site icon, and ran in to an issue with multisites. When using the get_site_icon_url hook, like so, $blog_id always returns 0 in a multisite.

<?php
/**
 * Register favicon
 *
 * @param string $url
 * @param integer $size
 * @param integer $blog_id
 * @return string
 */
function mytheme_set_site_icon_url(string $url, int $size, int $blog_id): string {
    if (get_current_blog_id() === $blog_id) {
        return get_theme_file_uri("assets/media/logo-favicon-{$size}x{$size}.svg");
    }

    return $url;
}
add_filter("get_site_icon_url", "mytheme_set_site_icon_url", 10, 3);

Change History (8)

#1 @sabernhardt
18 months ago

  • Focuses multisite added

#2 @audrasjb
18 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.3
  • Version changed from 6.2 to 4.4

Hello @revxx14 and welcome to WordPress Core Trac

This was introduced with the hook in WP 4.4.
Moving for 6.3 consideration.

#3 @petitphp
18 months ago

Hi @revxx14

Looking at the code for get_site_icon_url function, I think it's the expected behavior. The value use by the filter is what was passed as a parameter to the function and not the current blog id.

The default value of $blog_id is 0, which mean "the current blog".

I'm not sure that I completely understand your condition, but in theory you could add a check to account for that case

<?php
/**
 * Register favicon
 *
 * @param string $url
 * @param integer $size
 * @param integer $blog_id
 * @return string
 */
function mytheme_set_site_icon_url(string $url, int $size, int $blog_id): string {
    if ( empty( $blog_id ) || get_current_blog_id() === $blog_id) {
        return get_theme_file_uri("assets/media/logo-favicon-{$size}x{$size}.svg");
    }

    return $url;
}
add_filter("get_site_icon_url", "mytheme_set_site_icon_url", 10, 3);

#4 @revxx14
18 months ago

@petitphp if I use the get_site_icon_url without adding any sort of check against $blog_id, all of the icons under "My Sites" in the admin toolbar break. My example is a little simplified, but the goal here is to use a different icon for each site in the multisite. The particular site that I'm trying to set this up on returns 7 for get_current_blog_id(), so I don't understand why $blog_id returns 0. Shouldn't that change for each site in the multisite?

#5 @petitphp
18 months ago

@revxx14

Yes it's confusing. Here $blog_id is not supposed to be equal to the current blog id, but to the blog id we are retrieving the site icon for.

Here are some example with the different cases :

<?php
// Assuming the examples are run on the blog `7`

// Case 1 : Retrieving the site icon url for the current site.
// Here the value of `$blog_id` will be `0` and the value of `get_current_blog_id` will be `7`
$icon_url = get_site_icon_url();

// Case 2 : Retrieving the site icon url for the blog `12`.
// Here the value of `$blog_id` will be `12`and the value of `get_current_blog_id` will be `7`
$icon_url = get_site_icon_url( 512, '', 12 );

// Case 3 : Retrieving the site icon url for the blog `7` (even if already on the blog `7`).
// Here the value of `$blog_id` will be `7`and the value of `get_current_blog_id` will be `7`
$icon_url = get_site_icon_url( 512, '', 7 );

The first case is the one used in WordPress core. That why you always have the value 0 for $blog_id when hooking the filter. If you see 0 it means we are getting the site icon url for the current site.

Here one possible way this could be handled

<?php
function mytheme_set_site_icon_url(string $url, int $size, int $blog_id): string {
    // If `$blog_id` is `0`, fill it with the current blog id
    if ( empty( $blog_id ) ) {
        $blog_id = get_current_blog_id();
    }

    // Load the desired icon url
    switch ( $blog_id ) {
        case 7:
            $url = "https://example.com/7/my/custom/icon-{$size}x{$size}.svg"
            break;
        //...
    }

    return $url;
}
add_filter("get_site_icon_url", "mytheme_set_site_icon_url", 10, 3);

#6 @revxx14
18 months ago

  • Resolution set to invalid
  • Status changed from new to closed

@petitphp Ah, I see, that does make a little more sense, though I'd still argue that returning 0 for this filter in the multisite context is very confusing and should be changed. I'm not the biggest fan of having to hard code a database value (the $blog_id value of 7), I'd hoped to be able to pull everything from the database automatically, but oh well, this will do. Thanks!

EDIT: Whoops, didn't mean to choose "invalid," not sure how to change that.

Last edited 18 months ago by revxx14 (previous) (diff)

#7 @petitphp
18 months ago

@revxx14

My example is really naive. It was more to show that the value 0 can be treated as the current blog. You certainly can pull info from the database if you need !

Last edited 18 months ago by petitphp (previous) (diff)

#8 @SergeyBiryukov
15 months ago

  • Keywords needs-patch removed
  • Milestone 6.3 deleted

Removing the milestone, as there were no changes to WordPress core here.

We could probably add a note to the filter documentation that the value of 0 for $blog_id means the current blog. Feel free to reopen or create a new ticket for that :)

Note: See TracTickets for help on using tickets.