Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

#61408 closed defect (bug) (fixed)

Site logo returns image attachment when no logo is defined

Reported by: greenshady's profile greenshady Owned by: joedolson's profile joedolson
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.6
Component: Themes Keywords: has-patch needs-testing has-testing-info commit
Focuses: Cc:

Description

This is a regression that seems to have cropped up from this change: https://core.trac.wordpress.org/changeset/58213

Ticket: https://core.trac.wordpress.org/ticket/60922

To reproduce, activate the Twenty Twenty-Four theme on your site but do not set a logo for the site. Ensure that the Site Logo block is in the header (it is by default).

View any page on the site, you shouldn't see a logo. View an image attachment page, the logo will use the attached image instead.

Attachments (5)

logo-attachment-incorrect.jpeg (203.5 KB) - added by greenshady 6 weeks ago.
Attachment image being used for site logo.
61408.diff (530 bytes) - added by joedolson 6 weeks ago.
Only execute wp_attachment_is_image if there is a value in site logo.
Patch testing.png (255.9 KB) - added by krupajnanda 6 weeks ago.
Screenshot at Jun 12 11-49-40 PM.png (23.5 KB) - added by hmbashar 6 weeks ago.
showing this error, doesn't work for me
61408.2.diff (545 bytes) - added by rajinsharwar 6 weeks ago.
Refreshed patch

Download all attachments as: .zip

Change History (11)

@greenshady
6 weeks ago

Attachment image being used for site logo.

#1 @joedolson
6 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.6
  • Owner set to joedolson
  • Status changed from new to accepted

The problem is that has_custom_logo() now uses wp_attachment_is_image(), which uses the $post global if no value is passed. Need to exit earlier in has_custom_logo().

@joedolson
6 weeks ago

Only execute wp_attachment_is_image if there is a value in site logo.

#2 @joedolson
6 weeks ago

  • Keywords has-patch needs-testing added; needs-patch removed

Thanks, @greenshady!

#3 @greenshady
6 weeks ago

Tested the patch. It's working for me.

#4 @krupajnanda
6 weeks ago

  • Keywords needs-testing-info added

Hey @greenshady

I was trying to test this fix. I am not pretty sure if I have followed the correct steps. But it would be helpful if you could point me to the right direction. Please check the attachment and what it says.

@hmbashar
6 weeks ago

showing this error, doesn't work for me

@rajinsharwar
6 weeks ago

Refreshed patch

#5 @rajinsharwar
6 weeks ago

  • Keywords has-testing-info commit added; needs-testing-info removed

Hi @hmbashar @krupajnanda, it seems that the lines upstream have changed compared to patch 61408. Please try this new one: 61408.2.diff

I have tested the fix myself, and it does seem to work.
Before: https://img001.prntscr.com/file/img001/0UfuioC6S52gevPD9b6e3A.png
After: https://img001.prntscr.com/file/img001/XnyPGihER4arXIVLxCbHOg.png

Note for testers: Please know that attachment pages are disabled by default from 6.4. So, you can visit wp-admin/options.php on your site, search for wp_attachment_pages_enabled, and change the option to 1 to enable the option for testing.

As it's a relatively small change, and already has two manual tests, marking this for commit.

#6 @joedolson
6 weeks ago

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

In 58407:

Themes: Fix attachment rendered as site logo on attachment page.

Fix issue where the attachment thumbnail would be rendered as the site logo on attachment single templates if no site logo is set. Avoid calling wp_attachment_is_image() with no value, since that function will fallback to the global $post variable. Follow up to [58213]. See #60922.

Props greenshady, krupajnanda, hmbashar, rajinsharwar, joedolson.
Fixes #61408.

Note: See TracTickets for help on using tickets.