WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#12735 closed defect (bug) (fixed)

get_blogaddress_by_* adds path in multisite. breaks get_home_url etc

Reported by: andreasnrb Owned by:
Milestone: 3.0 Priority: high
Severity: critical Version: 3.0
Component: Multisite Keywords:
Focuses: Cc:

Description

The get_blogaddress_by_* functions adds the wp install path to all returned blogaddresses. This breaks for example get_home_url which returns the equivalent of the install path instead.

Need to decide the correct behavior for these methods.

Attachments (4)

12736&12735.patch (8.7 KB) - added by andreasnrb 5 years ago.
Fixes get_blogaddress_*, adds siteurl,homeurl to currents sites and get_blogs, fixes wrong linkage in my-sites and ms
fix-get_blogadd-net_site-site_path.patch (4.9 KB) - added by andreasnrb 5 years ago.
Fixes get_blogaddress_*, network_site_url,get_site_url, get_home_url, adds site_path support, to currsite & ms-default-constants
12735.1.patch (5.8 KB) - added by andreasnrb 5 years ago.
Fixes get_blogaddress_*, network_site_url,get_site_url, get_home_url, adds site_path support, to currsite & ms-default-constants, fixes bug in previous patch and adds site url support to wpmu blog creation
12736.2.patch (1.5 KB) - added by andreasnrb 5 years ago.
Removed the site path stuff. Fixed the home_url and get_blogaddress_by_name bug

Download all attachments as: .zip

Change History (22)

comment:1 @andreasnrb5 years ago

This happens when you have installed wordpress in a subfolder.

comment:2 @andreasnrb5 years ago

get_site_url assumes get_blogaddress_by_id returns siteurl.
What should get_blogaddress_by return? siteurls or homeurls?

comment:3 @ryan5 years ago

I'm not sure, but I don't think MU really supports subdirectory installs. That would explain why it seemingly makes no distinction between home url and site url.

@andreasnrb5 years ago

Fixes get_blogaddress_*, adds siteurl,homeurl to currents sites and get_blogs, fixes wrong linkage in my-sites and ms

comment:4 follow-up: @ryan5 years ago

Line 71 of my-sites.php should use admin_url(). homeurl and siteurl in blog details seem pretty useless. We should almost always go through the *_url() functions to get those. Likewise, $current_site really doesn't need homeurl and siteurl. Just set site_path and let the network_url() do the piecing together. That way there a not multiple means of accessing the same thing.

comment:5 in reply to: ↑ 4 @andreasnrb5 years ago

Replying to ryan:

Line 71 of my-sites.php should use admin_url(). homeurl and siteurl in blog details seem pretty useless. We should almost always go through the *_url() functions to get those. Likewise, $current_site really doesn't need homeurl and siteurl. Just set site_path and let the network_url() do the piecing together. That way there a not multiple means of accessing the same thing.

my-sites cant use admin_url since that is for a single blog. if anything it should be get_admin_url and get_home_url but that just adds another function call to get the same info that really belongs to blogdetails. Siteurl in blogdetails wasnt added by me so don't think we can remove it. The blogdetails really should be loaded in one big swoop instead of individual calls like it is now.

Saw some bugs in my patch that I'll fix tomorrow.

comment:6 follow-up: @ryan5 years ago

Yes, get_admin_url(), which is scheme aware unlike blog details. siteurl must stay but we shouldn't add homeurl.

comment:7 in reply to: ↑ 6 ; follow-up: @andreasnrb5 years ago

Replying to ryan:

Yes, get_admin_url(), which is scheme aware unlike blog details. siteurl must stay but we shouldn't add homeurl.

okidoki. Ill do approriate fixes tomorrow then if no one has done it by then.

@andreasnrb5 years ago

Fixes get_blogaddress_*, network_site_url,get_site_url, get_home_url, adds site_path support, to currsite & ms-default-constants

comment:8 in reply to: ↑ 7 @andreasnrb5 years ago

Replying to andreasnrb:
Don't know how to delete a patch but the latest one is the one to use. (fix-get_blogadd-net_site-site_path.patch)

@andreasnrb5 years ago

Fixes get_blogaddress_*, network_site_url,get_site_url, get_home_url, adds site_path support, to currsite & ms-default-constants, fixes bug in previous patch and adds site url support to wpmu blog creation

comment:9 @wpmuguru5 years ago

Reading back through the meetup discussion and wanted to post a correction related to this ticket.

Multisite runs fine in a folder as long as it is installed in that folder. It doesn't support offsetting the home url into a folder under the install.

@andreasnrb5 years ago

Removed the site path stuff. Fixed the home_url and get_blogaddress_by_name bug

comment:10 @andreasnrb5 years ago

I'll be adding the wp install folder support stuff to #12814 . it requires changing the initial multisite activation so thought it best to not intermingle the problems.

comment:11 follow-up: @nacin5 years ago

We're not supporting siteurl != homeurl. So is this patch still current?

comment:12 in reply to: ↑ 11 @andreasnrb5 years ago

Replying to nacin:

We're not supporting siteurl != homeurl. So is this patch still current?

I'd like to think so. It makes the code coherent. And the 2nd part is actually a bug fix but a little crappy one. Wasn't 100% sure how path and domain worked when I wrote it. It works for me though. It should be a string comparison.

comment:13 @wpmuguru5 years ago

(In [14702]) improvements to get_home/site_url, props andreasnrb + code cleanup, see #12735

comment:14 @wpmuguru5 years ago

(In [14704]) use get_blogaddress_by_name in wp-signup, see #12735

comment:15 @wpmuguru5 years ago

(In [14705]) more use get_blogaddress_by_name, see #12735

comment:16 @wpmuguru5 years ago

That last one should have been network_home_url() in the comment.

comment:17 @wpmuguru5 years ago

AFAIK, this one is fixed.

comment:18 @ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.