Opened 3 years ago

Last modified 4 days ago

#15337 new defect (bug)

fix get_attachment_template() to pass templates array

Reported by: willnorris Owned by:
Priority: normal Milestone: Future Release
Component: Template Version: 2.0
Severity: normal Keywords: dev-feedback has-patch
Cc: johnbillion@…, chip@…, jeff@…

Description

get_attachment_template() currently queries for a number of templates based on the mime-type of the attachment. It does so by checking each one individually, and returning as soon as it finds one. It should instead build an array of templates, and pass the entire array to get_query_template().

patch attached.

Attachments (5)

16230.diff (842 bytes) - added by willnorris 3 years ago.
template.php.diff (962 bytes) - added by jfarthing84 11 months ago.
Reorder attachment template hierarchy, change underscore to dash in most specific scenario and pass all templates as an array.
15337.patch (1.1 KB) - added by SergeyBiryukov 5 months ago.
15337.2.patch (1.3 KB) - added by SergeyBiryukov 5 months ago.
15337.3.patch (1.5 KB) - added by SergeyBiryukov 5 months ago.

Download all attachments as: .zip

Change History (20)

to be honest, I also think that order it attempts the templates is completely backwards. For an "image/jpeg" attachment, it currently attempts:

  • image.php
  • jpeg.php
  • image_jpeg.php
  • attachment.php

the mime-type portion of this list is going from general to specific... shouldn't it instead start by looking for the most specific template, and then back off to the most general? so ...

  • image_jpeg.php
  • jpeg.php
  • image.php
  • attachment.php

Now I realize this is not something that can be easily changed since it could potentially break a number of themes. Though I'd be highly surprised if there are that many themes that have different templates for "image.php" and "image_jpeg.php".

perhaps this should be filed as a separate bug so it can be discussed, and not hold up this other patch?

  • Milestone changed from Awaiting Review to 3.1

Reversing the order has been mentioned in previous tickets, but I don't think we should bother with thar.

Patch makes sense.

comment:3 follow-up: ↓ 14   greenshady3 years ago

Note that the patch will break older hooks:

  • $type[0]_template
  • $type[1]_template
  • $type[0]_$type[1]_template

We'll only be left with the attachment_template hook. I actually think this makes more sense anyway because of the addition of the $type_template_hierarchy hook added to the get_query_template()function.

I don't think the change would really break compatibility with many (if any) themes/plugins though.

  • Cc has-patch removed
  • Keywords 2nd-opinion added
  • Milestone changed from 3.1 to Future Release
  • Cc johnbillion@… added
  • Cc chip@… added
  • Cc jeff@… added

Related #21053.

  • Keywords dev-feedback added; 2nd-opinion removed

comment:9 follow-up: ↓ 11   jfarthing8411 months ago

Also just noticed that with the most specific template, the intended result isn't even used. For instance, a JPEG image would be image_jpeg.php. However, get_query_template uses a regular expression that removes the underscore, thus changing the template it looks for to imagejpeg.php. I suggest that the underscore be changed to a dash, thus matching the WordPress file naming convention and avoiding this problem.

Reorder attachment template hierarchy, change underscore to dash in most specific scenario and pass all templates as an array.

  • Keywords has-patch added

comment:11 in reply to: ↑ 9   SergeyBiryukov11 months ago

Replying to jfarthing84:

However, get_query_template uses a regular expression that removes the underscore, thus changing the template it looks for to imagejpeg.php.

Related: #21213

#23151 was marked as a duplicate.

  • Component changed from Themes to Template
  • Milestone changed from Future Release to 3.6
  • Version set to 2.0

Two refreshed patches:

  • 15337.patch keeps the backwards compatibility, except for the underscore issue in comment:9 (i.e. image_jpeg.php, which previously didn't work, would now do).
  • 15337.2.patch reverses the order per comment:1 and changes the underscore to a dash.

Both patches also use get_queried_object() instead of the $posts global for consistency with other get_*_template() functions.

comment:14 in reply to: ↑ 3   SergeyBiryukov5 months ago

15337.3.patch is an attempt to keep backwards compatibility for filters, per comment:3.

comment:15   ryan4 days ago

  • Milestone changed from 3.6 to Future Release
Note: See TracTickets for help on using tickets.