WordPress.org

Make WordPress Core

Opened 8 weeks ago

Last modified 8 weeks ago

#48149 new enhancement

Suggestion: Optional parameter to remove width and height attribute from attachment image markup

Reported by: subrataemfluence Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.2.3
Component: Media Keywords: has-patch 2nd-opinion
Focuses: Cc:
PR Number:

Description

First of all I am sorry if this had already been addressed and enhanced.
I am on 5.2.3.

Like post_thumbnail_html and image_send_to_editor filters I was unable to find one for attachment images for posts.

If we can add an additional Boolean type argument in wp_get_attachment_image function (/wp-includes/media.php, line 872) in the following manner and create $hwstring string based on this argument's value, it will give us the control to decide whether or not to render those two attributes.

Here is my suggestion:

<?php
function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $render_hw = false, $icon = false, $attr = '' ) {
     ...
     $hwstring = $render_hw ? image_hwstring( $width, $height ) : '';
     ...
}

The default function call will be:

wp_get_attachment_image( $att->ID,  'infotravel-post-page-thumb' );

And, if width and height attributes need to be added, the function call will change to:

wp_get_attachment_image( $att->ID, true,  'infotravel-post-page-thumb' );

There are two reasons why I am proposing this:

  1. Responsiveness. Image width is controlled from CSS and media-queries
  2. The function already accepts a $size. Since we have the ability to pass a specific known size, we do not need to add width and height attributes.

Based on the above approach I have proposed a patch. Please let me know whether it is useful!

Attachments (2)

48149.diff (1.5 KB) - added by subrataemfluence 8 weeks ago.
Proposed patch
48149-2.diff (1.4 KB) - added by subrataemfluence 7 weeks ago.
New parameter has been pushed to the end of current argument list.

Download all attachments as: .zip

Change History (4)

@subrataemfluence
8 weeks ago

Proposed patch

#1 @SergeyBiryukov
8 weeks ago

Previously: #14110, #20358, #30525.

Thanks for the patch! Just a quick note that new parameters cannot be added in the middle of an existing function signature, as that breaks backward compatibility. They can only be added at the end.

It would also be good to revisit the discussions from the previous tickets linked above.

#2 @subrataemfluence
8 weeks ago

It was my mistake to put the new argument in middle of existing function signature. It has to be at the end. Thank you for pointing out.

I have gone through the discussions in earlier tickets and here is my observation (I may be wrong):

#14110: The patches suggested there use a lot of code modification which to me looks unnecessary.

#20358 and #30525 both are suggesting a filter. But I think an additional filter for this purpose might not be a good idea, specially when just adding a single parameter (at the end of current argument list of course!) is sufficient to handle the situation.

Please let me know whether I may upload a modified patch putting the new argument at the end of current list.

Last edited 8 weeks ago by subrataemfluence (previous) (diff)

@subrataemfluence
7 weeks ago

New parameter has been pushed to the end of current argument list.

Note: See TracTickets for help on using tickets.