WordPress.org

Make WordPress Core

Opened 23 months ago

Last modified 20 months ago

#37297 reviewing enhancement

Deprecate get_blogaddress_by_id function

Reported by: spacedmonkey Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: needs-patch
Focuses: multisite Cc:

Description

The get_blogaddress_by_id function is pretty old and should be replaced with get_home_url, as this is filtered.

Attachments (1)

37297.diff (3.0 KB) - added by spacedmonkey 23 months ago.

Download all attachments as: .zip

Change History (12)

@spacedmonkey
23 months ago

#1 @thomaswm
23 months ago

You cannot replace get_blogaddress_by_id with get_home_url in install_blog(), because the home option hasn't been defined yet at that point. It is set in line 1360 in ms-functions.php.

So, in install_blog(), you'll have to do something like

$site = get_site( $blog_id );
$url = 'http://' . $site->domain . $site->path;

to get the value of $url.

#2 @spacedmonkey
22 months ago

  • Keywords has-patch 4.7-early added

#3 @jeremyfelt
21 months ago

  • Keywords 4.7-early removed
  • Milestone changed from Awaiting Review to 4.7
  • Owner set to jeremyfelt
  • Status changed from new to reviewing

Noting #26855 in here as well. Let's see if we can deprecate this early. :)

#4 @jorbin
20 months ago

Early is coming to a an end. (whatever early means). @jeremyfelt - Think this can go in, or do you need to do #26855 first?

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


20 months ago

#6 @jeremyfelt
20 months ago

  • Keywords needs-patch added; has-patch removed
  • Owner jeremyfelt deleted

I spent some time with 37297.diff the other day and ran into some uncertainty.

As @thomaswm mentioned, get_home_url() is not appropriate solution for install_blog(). get_blogaddress_by_id() attempts to use the home option to determine the URL's scheme, but it will always be empty at this point. We then know it will be HTTP combined with the site's domain and path.

It does seem to be the right option for wpmu_welcome_notification(). I'd like this to be tested more though.

The use in wp-activate.php has already been removed via #26855.

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


20 months ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


20 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


20 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


20 months ago

#11 @flixos90
20 months ago

  • Milestone changed from 4.7 to Future Release

This needs further thought and should be tested earlier in a release cycle. Let's reconsider it in one of the next releases.

Note: See TracTickets for help on using tickets.