WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 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 10 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 10 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 10 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 10 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)

#1 @andreasnrb
10 years ago

This happens when you have installed wordpress in a subfolder.

#2 @andreasnrb
10 years ago

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

#3 @ryan
10 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.

@andreasnrb
10 years ago

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

#4 follow-up: @ryan
10 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.

#5 in reply to: ↑ 4 @andreasnrb
10 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.

#6 follow-up: @ryan
10 years ago

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

#7 in reply to: ↑ 6 ; follow-up: @andreasnrb
10 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.

@andreasnrb
10 years ago

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

#8 in reply to: ↑ 7 @andreasnrb
10 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)

@andreasnrb
10 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

#9 @wpmuguru
10 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.

@andreasnrb
10 years ago

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

#10 @andreasnrb
10 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.

#11 follow-up: @nacin
10 years ago

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

#12 in reply to: ↑ 11 @andreasnrb
10 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.

#13 @wpmuguru
10 years ago

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

#14 @wpmuguru
10 years ago

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

#15 @wpmuguru
10 years ago

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

#16 @wpmuguru
10 years ago

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

#17 @wpmuguru
10 years ago

AFAIK, this one is fixed.

#18 @ryan
10 years ago

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