allowed_themes filter does not pass blog_id

[rmccue]

The allowed_themes does not pass the $blog_id parameter into the filter, so you can't currently filter based on the site.

(There's also no filters in WP_Theme::get_allowed_on_network or WP_Theme::get_allowed_on_site, plus the two arrays are combined together, so a filter just in WP_Theme::get_allowed_on_site would be inadequate for blacklisting themes for certain sites.)

[DrewAPicture]

Would you care to create a patch for the former? :-)

[nacin]

I'd have to go back into the ticket and older code to jog my memory on this, but the current 'allowed_themes' filter is likely explicitly applied to the network list of themes only, for backwards compatibility reasons. (It's an old filter.) Expanding it to what is allowed on a site could be problematic, depending on how people are using it.

Off the top of my head, we should probably shift the 'allowed_themes' filter into ::get_allowed_on_network() and add a new filter to get_allowed().

[rmccue]

Replying to nacin:

I'd have to go back into the ticket and older code to jog my memory on this, but the current 'allowed_themes' filter is likely explicitly applied to the network list of themes only, for backwards compatibility reasons. (It's an old filter.)

Indeed, @since MU

Off the top of my head, we should probably shift the 'allowed_themes' filter into ::get_allowed_on_network() and add a new filter to get_allowed().

Agreed. :) We should add a filter to get_allowed_on_site as well as one for the combined value from get_allowed.

[jeremyfelt]

I would enjoy this filter.

[jeremyfelt]

Naming may get a bit confusing here as allowed_themes moves to get_allowed_on_network() and something new is needed for get_allowed() and get_allowed_on_site(). It all makes sense though.

[pauldewouters]

Added a patch adding the filters. Also uploaded wrong file by mistake. How do I delete it?

[DrewAPicture]

Replying to pauldewouters:

Added a patch adding the filters. Also uploaded wrong file by mistake. How do I delete it?

It's alright, just upload your new patch.

[posungucci]

screenshot

[pauldewouters]

Replying to DrewAPicture:

Replying to pauldewouters:

Added a patch adding the filters. Also uploaded wrong file by mistake. How do I delete it?

It's alright, just upload your new patch.

The correct patch is 28436.diff

[jeremyfelt]

More background in #21099 and [21686]

[lamosty]

Pass $blog_id parameter into the filter and create unit test.

[lamosty]

We are having a Contributors Day in Slovakia. As a team, we worked on a simple unit test for this ticket.

[pento]

Nice! There are a few code style and comments that need tweaking:

• The docs for the allowed_themes_combined and allowed_themes_site filters don't include $blog_id.
• The @since for the allowed_themes filter should stay as "MU", not be updated to "4.2" (we can set the other filter versions correctly when this patch is accepted into core).
• In the unit tests, you're backing up the $wpdb->suppress_errors() state, but this doesn't appear to be changed anywhere.
• Is there any particular reason for setting $_SERVER['REMOTE_ADDR'] in the unit tests?

For props purposes, would you mind listing who is on the team at the Contributor Day?

[pento]

Oh, and while I'm a big fan of closures, we do still need our unit tests to run in PHP 5.2. The unit test will need to run with a defined method.

[lamosty]

Resolved issues mentioned by @pento.

[lamosty]

@pento thanks for the feedback, really appreciate that.

Point 3 and 4: I (carelessly) copied another unit test to create this one. Sorry.

List of the team (in no particular order):

• michalzuber
• dmsnell
• johnnypea
• rob

[fliespl]

#32371 was marked as a duplicate.

[swissspidy]

Current patch looks good, just needs a small update:

• Needs to use @since 4.5.0 in the DocBlock (trailing zero)
• 'allowed_themes' filter is @since MU
• setUp() / tearDown() in the test class are not needed when they just call the parent's method

[jeremyfelt]

In 36350:

Themes: Add initial tests for the allowed_themes filter.

We'll be adjusting the placement of this filter and adding two other related filters, so we should make sure it continues to work as expected after the change.

See #28436.

[jeremyfelt]

Almost. :)

Any naming ideas for allowed_themes_combined? I'd like something a little less confusing.

We could use allowed_themes_network, but that may be confusing as allowed_themes will be the legacy filter in WP_Theme::get_allowed_on_network().

Other adjustments in 28436.4.diff​:

• There are two return points for the allowed_themes_site filter in WP_Theme::get_allowed_on_site().
• The allowed_themes filter needs to be on the return of WP_Theme::get_allowed_on_network() rather than inside the isset() check, otherwise it will not always fire.

[jeremyfelt]

In 28436.5.diff​ using these:

• allowed_themes is legacy and used to filter network allowed themes directly via WP_Theme::get_allowed_on_network().
• network_allowed_themes is new in WP_Theme::get_allowed() and filters what is received from WP_Theme::get_allowed_on_network() before combining that result with site results.
• site_allowed_themes can be used to filter site allowed themes via WP_Theme::get_allowed_on_site(). These results are combined with network allowed themes.

[jeremyfelt]

In 36366:

Themes: Enhance filtering options for allowed themes on a network.

• Move the legacy allowed_themes filter to WP_Theme::get_allowed_on_network(), where it will continue to filter themes allowed on the network.
• Add network_allowed_themes filter to WP_Theme::get_allowed() and pass $blog_id to provide context.
• Add site_allowed_themes filter to WP_Theme::get_allowed_on_site() and pass $blog_id to provide context.

Props pauldewouters, lamosty, michalzuber, dmsnell, johnnypea, rob.
Fixes #28436.