Make WordPress Core

Opened 2 months ago

Closed 2 months ago

#60740 closed defect (bug) (fixed)

wp_mime_type_icon function fails to return non-SVG icons

Reported by: dglingren's profile dglingren Owned by: joedolson's profile joedolson
Milestone: 6.5 Priority: normal
Severity: normal Version: 6.5
Component: Media Keywords: has-patch has-testing-info dev-reviewed
Focuses: Cc:

Description

WordPress 6.5 added a new parameter, $preferred_ext, to the wp_mime_type_icon function in the /wp-includes/post.php file. It is described as "File format to prefer in return." In fact, it is the REQUIRED extension, not the preferred extension, and no other file extension is allowed. WordPress passes ".svg" in the argument and that prevents any other file type, e.g., a ".png" icon, from being returned.

You can reproduce the problem by activating a plugin that supplies a custom icon directory with non-SVG icon files, such as my own Media Library Assistant. Simply install and activate the plugin, then go to the Settings/Media Library Assistant Uploads tab. Check the "Enable MLA File Type Icons Support" box and click Save Changes. The go to the Media/Library or Media/Assistant admin screen and display a non-image item such as an audio or video item.

When this bug occurs the wp_mime_type_icon function causes a PHP Warning message. "Undefined variable $icon in ...\wp-includes\post.php on line 6940", and the function returns a NULL value. The function also caches an empty array for the $icon_files variable so subsequent function calls return a false value.

Change History (14)

#1 @joedolson
2 months ago

  • Keywords needs-patch added
  • Owner set to joedolson
  • Status changed from new to accepted
  • Version set to trunk

Thanks for reporting!

#2 @joedolson
2 months ago

  • Milestone changed from Awaiting Review to 6.5

This ticket was mentioned in PR #6249 on WordPress/wordpress-develop by @joedolson.


2 months ago
#3

  • Keywords has-patch added; needs-patch removed

Return the array of all icons found in icon directory if the preferred extension is not found.

Trac ticket: https://core.trac.wordpress.org/attachment/ticket/60740

#4 @joedolson
2 months ago

  • Keywords has-testing-info needs-testing added

To test:

  • Follow testing instructions provided by @dglingren. Observe the error or non-loading icons before patch, note properly loading icon after patch.
  • Deactivate plugin and verify that the .svg icons appear as expected both before and after patch.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


2 months ago

This ticket was mentioned in PR #6269 on WordPress/wordpress-develop by @joedolson.


2 months ago
#6

Properly based version of prior PR

Trac ticket: https://core.trac.wordpress.org/ticket/60740

#7 @antpb
2 months ago

  • Keywords commit added; needs-testing removed

I left an approval in the GitHub issue but echoing here that this working in my testing following the reproduction steps and the code looks good.

#8 @joedolson
2 months ago

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

In 57845:

Media: Fall back to available icons if SVG media icons not found.

Follow up to [57687]. If no icons of the preferred type are available, then the icon array should return the collection of valid icons found, rather than an empty array.

Props sabernhardt, swissspidy, sabernhardt, antpb, joedolson.
Fixes #60740.

#9 @joedolson
2 months ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for committing to 6.5 branch.

#10 @sabernhardt
2 months ago

I added dglingren's props manually.

#11 @joedolson
2 months ago

  • Keywords dev-feedback added

Thanks, @sabernhardt.

I *know* I added them when I wrote the commit message; but I must have accidentally pasted yours over them. Argh.

#12 @joedolson
2 months ago

I'll add them in the commit to the 6.5 branch, though.

#13 @swissspidy
2 months ago

  • Keywords dev-reviewed added; dev-feedback removed

#14 @joedolson
2 months ago

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

In 57846:

Media: Fall back to available icons if SVG media icons not found.

Follow up to [57687]. If no icons of the preferred type are available, then the icon array should return the collection of valid icons found, rather than an empty array.

Props dglingren, sabernhardt, swissspidy, sabernhardt, antpb, joedolson.
Reviewed by swissspidy.
Merges [57845] to the 6.5 branch.
Fixes #60740.

Note: See TracTickets for help on using tickets.