WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#31428 closed defect (bug) (fixed)

Switch_to_blog() no longer works in Customize using Multisite.

Reported by: csschris Owned by: ocean90
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.1.1
Component: Customize Keywords: has-patch fixed-major
Focuses: multisite Cc:

Description

A change in /wp-includes/class-wp-customize-setting.php seems to have broken something.

Code that uses switch_to_blog() no longer gets run in customize view.

Had no problem in 4.1

Change History (25)

#1 follow-up: @ocean90
5 years ago

  • Focuses ui administration removed
  • Keywords reporter-feedback added

Can you please provide the code which is broken now?

#2 in reply to: ↑ 1 @csschris
5 years ago

Replying to ocean90:

Can you please provide the code which is broken now?

I'm in the process of narrowing it down.

seems like the

_preview_filter() function on line 167 /wp-includes/class-wp-customize-setting.php

Last edited 5 years ago by csschris (previous) (diff)

#3 follow-up: @westonruter
5 years ago

Code that uses switch_to_blog() no longer gets run in customize view.

Please share this code.

#4 in reply to: ↑ 3 @csschris
5 years ago

Replying to westonruter:

Code that uses switch_to_blog() no longer gets run in customize view.

Please share this code.

eg code could be

switch_to_blog(1);
echo bloginfo('name');
restore_current_blog();

shows current blog name not the blogname for site with id of 1

#5 follow-up: @westonruter
5 years ago

OK, then it is probably due to #30988 and this logic added to WP_Customize_Setting::preview():

if ( ! isset( $this->_original_value ) ) {
	$this->_original_value = $this->value();
}

Since the preview() method gets evaluated before the switch_to_blog() call, it would the later prevent the switch_to_blog() call from overriding this initial value.

However… is the plugin code intending to allow the Customizer to change the blogname for another site? Or are you just wanting to display the blogname for another site? If the latter case, then we probably need to update the _preview_filter method from modifying the value if the current blog is not the same as the initial blog when the preview method is called.

#6 in reply to: ↑ 5 @csschris
5 years ago

Replying to westonruter:

OK, then it is probably due to #30988 and this logic added to WP_Customize_Setting::preview():

if ( ! isset( $this->_original_value ) ) {
	$this->_original_value = $this->value();
}

Since the preview() method gets evaluated before the switch_to_blog() call, it would the later prevent the switch_to_blog() call from overriding this initial value.

However… is the plugin code intending to allow the Customizer to change the blogname for another site? Or are you just wanting to display the blogname for another site? If the latter case, then we probably need to update the _preview_filter method from modifying the value if the current blog is not the same as the initial blog when the preview method is called.

In that example I just want to show the name of the other blog. My actual use case is I show a menu from the main site (blog id 1) on all sub sites.

#7 follow-up: @westonruter
5 years ago

  • Owner set to westonruter
  • Status changed from new to assigned

But to confirm, you aren't intending to be able to modify that other blog's menu (or any other setting) from the current blog, right? I can understand wanting to use switch_to_blog to grab content for display within the Customizer preview, but using the Customizer to actually modify settings associated with other blogs would be a whole different animal.

#8 in reply to: ↑ 7 @csschris
5 years ago

Replying to westonruter:

But to confirm, you aren't intending to be able to modify that other blog's menu (or any other setting) from the current blog, right? I can understand wanting to use switch_to_blog to grab content for display within the Customizer preview, but using the Customizer to actually modify settings associated with other blogs would be a whole different animal.

Definitely not modifying other sites menu through Customizer. Just displaying a menu from another site which worked fine in 4.1 and below.

#9 follow-up: @westonruter
5 years ago

Cool. @csschris would you please take a look at 31428.diff and see if it resolves your issue?

#10 in reply to: ↑ 9 @csschris
5 years ago

Replying to westonruter:

Cool. @csschris would you please take a look at 31428.diff and see if it resolves your issue?

Thx @westonruter it does in fact resolve the issue.

#11 @csschris
5 years ago

  • Keywords reporter-feedback removed

#12 @westonruter
5 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.1.2
  • Owner changed from westonruter to ocean90
  • Status changed from assigned to reviewing

In 31428.2.diff"

  • Have WP_Customize_Setting::is_current_blog_previewed() return null if not previewed yet
  • Add unit test for WP_Customize_Setting::is_current_blog_previewed()
  • Clean up php coding standards in code around patch.

#13 @csschris
5 years ago

A note for recreating this bug.

Before @westonruter's fix this bug was specifically showing itself when using switch_to_blog then pulling menus by location and/or using has_nav_menu() to conditionally display content from other blogs.

I was confused because in another section I call switch_to_blog and then call a menu directly by ID without checking has_nav_menu and it did not break in customizer view in 4.1.1

#14 follow-up: @ocean90
5 years ago

  • Keywords has-patch added; commit removed

31428.2.diff:

  • For 4.1.2 we need a patch without a PHP coding standards clean up.
  • test_previewing_with_switch_to_blog() should either be split into two tests (one for single, one for multisite) or $this->markTestSkipped() should be on top.

#15 in reply to: ↑ 14 @westonruter
5 years ago

Replying to ocean90:

31428.2.diff:

  • For 4.1.2 we need a patch without a PHP coding standards clean up.
  • test_previewing_with_switch_to_blog() should either be split into two tests (one for single, one for multisite) or $this->markTestSkipped() should be on top.

OK, 31428.3.diff now addresses these.

@westonruter
5 years ago

Additional change (Add markTestSkipped to test_previewing_with_switch_to_blog if not is_multisite): https://github.com/xwp/wordpress-develop/commit/097e1f88e8a5b12ec3947d8b83e8680214f64ce0

#16 @ocean90
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 31707:

Customizer: Return the original value when filtering theme mods/options and the current blog has changed.

props westonruter.
fixes #31428.

#17 @ocean90
5 years ago

  • Keywords fixed-major added

Reopening for 4.1.2.

#18 @ocean90
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It's late…

#19 @nicholas_io
5 years ago

I have a multisite installation that performs a lot of switch_to_blog() on the main site to pull posts for another site and it`s working well in customizer preview with 4.1.1.

#20 @pento
5 years ago

  • Milestone changed from 4.1.2 to 4.2
  • Resolution set to fixed
  • Status changed from reopened to closed

With 4.2 being just around the corner, we're going to skip this for 4.1.2.

#21 @boonebgorges
5 years ago

In 32840:

Use assertFalse() rather than assertNull() in Tests_WP_Customize_Setting::test_is_current_blog_previewed().

is_current_blog_previewed() returns a boolean.

See #31428.

Note: See TracTickets for help on using tickets.