WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#36639 closed enhancement (fixed)

Customize: get_custom_logo filter should include blog parameter

Reported by: achbed Owned by: chriscct7
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.5
Component: Customize Keywords: has-patch
Focuses: Cc:
PR Number:

Description

The get_custom_logo function potentially performs a blog context switch in order to render the custom logo. However, the get_custom_logo filter is not called within that context switch, and the filter handlers have no way of knowing if a blog context switch was requested or required.

This patch solves both problems by providing the blog ID parameter to the get_custom_logo filter as well as moving the filter call so that it is in the same blog context.

Attachments (2)

custom-logo-blog_context.patch (805 bytes) - added by achbed 4 years ago.
36639.2.patch (549 bytes) - added by juanfra 4 years ago.
Patch without modifying the restore_current_blog() position

Download all attachments as: .zip

Change History (13)

#1 @westonruter
4 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.6
  • Version changed from trunk to 4.5

Good one. Makes sense to me!

#2 @ocean90
4 years ago

Yep, makes sense and would be consistent with get_site_icon_url. But let's not change the restore_current_blog() part.

#3 @achbed
4 years ago

The patch doesn't modify the restore_current_blog() call itself, just the lines before and after. Those lines got included by svn diff automagically.

@juanfra
4 years ago

Patch without modifying the restore_current_blog() position

#4 follow-up: @achbed
4 years ago

Should we not do the get_custom_logo filtering while we're in the switched blog scope? That would allow more consistent results if the alternate blog has additional filtering wouldn't it?

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


4 years ago

#6 in reply to: ↑ 4 ; follow-up: @flixos90
4 years ago

Replying to achbed:

Should we not do the get_custom_logo filtering while we're in the switched blog scope? That would allow more consistent results if the alternate blog has additional filtering wouldn't it?

The filter is currently applied outside of the switched blog scope, so we should stick with this for backwards compatibility in case someone is using the filter at this point. Applying the filter inside the switched scope could cause inconsistencies with whatever logic is executed in the filter functions.

Another reason to apply the filter outside of the switched scope is to be consistent with get_site_icon_url, as stated above.

#7 in reply to: ↑ 6 @achbed
4 years ago

Replying to flixos90:

Replying to achbed:

Should we not do the get_custom_logo filtering while we're in the switched blog scope? That would allow more consistent results if the alternate blog has additional filtering wouldn't it?

The filter is currently applied outside of the switched blog scope, so we should stick with this for backwards compatibility in case someone is using the filter at this point. Applying the filter inside the switched scope could cause inconsistencies with whatever logic is executed in the filter functions.

Another reason to apply the filter outside of the switched scope is to be consistent with get_site_icon_url, as stated above.

That makes sense. I concur - let's go with 36639.2.patch then. Thanks for the input (and thanks @juanfra for working that up)!

#8 @juanfra
4 years ago

@achbed my pleasure :)

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


3 years ago

#10 @chriscct7
3 years ago

  • Owner set to chriscct7
  • Status changed from new to reviewing

Docbloc needs an update to mention parameter since 4.6

#11 @ocean90
3 years ago

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

In 37645:

Template: Pass $blog_id to the get_custom_logo filter.

Props achbed, juanfra.
Fixes #36639.

Note: See TracTickets for help on using tickets.