Make WordPress Core

Opened 15 years ago

Closed 4 months ago

Last modified 3 months ago

#14110 closed enhancement (fixed)

Expose height and width attributes to 'wp_get_attachment_image_attributes' filter

Reported by: divinenephron's profile divinenephron Owned by: adamsilverstein's profile adamsilverstein
Milestone: 6.8.2 Priority: normal
Severity: minor Version: 3.0
Component: Media Keywords: has-patch commit has-unit-tests has-test-info fixed-major dev-reviewed
Focuses: Cc:

Description

The filter 'wp_get_attachment_image_attributes' allows you to alter the attributes of embedded images. However the height and width attributes aren't passed to this filter. These would be useful to have – I'm making a theme with a fluid layout where I have to remove all height and width attributes to ensure that the browser maintains the attribute of images when they're resized.

I've attached a patch with a fix. In it I've also changed the function 'get_image_tag' so that I could remove the immensely pointless 'hwstring' function.

Attachments (4)

wp_get_attachment_image.diff (3.4 KB) - added by divinenephron 15 years ago.
Patch to give the 'wp_get_attachment_image_attributes' filter access to the heigh and width attribues.
wp_get_attachment_image_shortened.diff (1.1 KB) - added by divinenephron 15 years ago.
Only the changed that are essential to the enhancement.
wp_get_attachment_image.2.diff (4.1 KB) - added by Sam_a 14 years ago.
Let plugins filter image dimensions via wp_get_attachment_image_attributes filter (*props divinenephron*). Restore image dimensions if they're missing after applying the filter (consistent with the wp_get_attachment_image()'s previous behavior). Pass $icon to the filter too. Move esc_attr closer to output. Update documentation. Whitespace.
wp_get_attachment_image.3.diff (4.1 KB) - added by wpsmith 14 years ago.
Minor modification to lines 695-6, restore null as empty strings

Download all attachments as: .zip

Change History (60)

@divinenephron
15 years ago

Patch to give the 'wp_get_attachment_image_attributes' filter access to the heigh and width attribues.

#1 @divinenephron
15 years ago

  • Keywords has-patch added

#2 @nacin
15 years ago

  • Keywords has-patch filter wp_get_attachment_image_attributes attribute height width attachment removed

I see no reason to move around so much code for such a simple request.

@divinenephron
15 years ago

Only the changed that are essential to the enhancement.

#3 @divinenephron
15 years ago

  • Cc divinenephron added
  • Component changed from Plugins to Media
  • Keywords has-patch filter wp_get_attachment_image_attributes attribute height width attachment added
  • Severity changed from normal to minor

Evidently it's rude to refactor code in an enhancement request – sorry about that.
I've attached "wp_get_attachment_image_shortened.diff" – a file that contains only the changes that are essential to the enhancement. Is that more acceptable.

#4 @nacin
15 years ago

  • Keywords filter wp_get_attachment_image_attributes attribute height width attachment removed
  • Milestone changed from Awaiting Review to 3.1

Sorry, wasn't trying to be rude. It's just really hard to see what's going on when there's a ton of unrelated red and green.

This code looks fine, aside from the coding whitespace. (We use tabs, not spaces.)

#5 follow-ups: @nacin
15 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from 3.1 to Future Release

I think this has the potential to break the filter, for those who receive an array through wp_get_attachment_image_attributes and build their own, not expecting additional keys.

#6 @scottconnerly
14 years ago

  • Cc scottconnerly added

I too would like to be able to access the image's height/width information from within the wp_get_attachment_image_attributes filter.

#7 @Sam_a
14 years ago

+1. When I added the wp_get_attachment_image_attributes hook (#8732), I omitted the height and width attributes because wp_get_attachment_image() had used image_hwstring() to format those. But it really doesn't make any sense that they're left out.

Last edited 14 years ago by Sam_a (previous) (diff)

#8 in reply to: ↑ 5 @Sam_a
14 years ago

Replying to nacin:

I think this has the potential to break the filter, for those who receive an array through wp_get_attachment_image_attributes and build their own, not expecting additional keys.

That's true. I wonder how common that is.

@Sam_a
14 years ago

Let plugins filter image dimensions via wp_get_attachment_image_attributes filter (*props divinenephron*). Restore image dimensions if they're missing after applying the filter (consistent with the wp_get_attachment_image()'s previous behavior). Pass $icon to the filter too. Move esc_attr closer to output. Update documentation. Whitespace.

#9 in reply to: ↑ 5 @Sam_a
14 years ago

Replying to nacin:

I think this has the potential to break the filter, for those who receive an array through wp_get_attachment_image_attributes and build their own, not expecting additional keys.

Attached wp_get_attachment_image.2.diff should cover that.

Patch may look bigger than it is. ;)

@wpsmith
14 years ago

Minor modification to lines 695-6, restore null as empty strings

#10 @wpsmith
14 years ago

Wouldn't it make more sense to "restore" height/width ($attr['width']/$attr['height']) attributes as empty strings rather than their full value?

Last edited 14 years ago by wpsmith (previous) (diff)

#11 @Sam_a
14 years ago

Hi wpsmith,

Under WordPress's current behavior, the width and height attributes don't go through the wp_get_attachment_image_attributes filter, so a plugin that builds its own $attr array and returns it without width and height doesn't actually intend to remove them (as nacin noted in #5).

So if we also expose width and height in the filter, we want to continue that behavior — if the width and height keys are missing after applying the filter, restore them to their original values, as if they hadn't been removed.

Plugins that really want to empty width and height can set them to '' or false through the filter to get that.

#12 @anatolbroder
13 years ago

  • Cc anatol@… added

#13 @anatolbroder
13 years ago

Maybe you should combine the functions get_attachment_image and get_image_tag. They try to do similar things, but are both not flexible enough for using in plugins.

We need one low level function lowlevel_image for just creating an image tag from an attribute array.

<img alt='Image' class='image' data-image='{JSON}' height='456' id='123' src='image.png' title='Image' width='789' … /> 

And we need a high level function highlevel_image for own attachment images. The highlevel_image should collect the attributes from image meta an pass them to the lowlevel_image.

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


12 years ago

#15 @ericlewis
11 years ago

  • Keywords reporter-feedback added; has-patch 2nd-opinion removed
  1. Why were we setting the image width/height attributes to begin with?
  1. Why would you need to override this for responsive design? max-width: 100% (or something like it) doesn't suffice?

#16 @wonderboymusic
11 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

No reporter feedback in 4 months.

#17 @SergeyBiryukov
11 years ago

#30525 was marked as a duplicate.

#18 @hereswhatidid
11 years ago

+1. Is this being closed simply due to lack of feedback?

#19 @johnbillion
8 years ago

#20358 was marked as a duplicate.

#20 @dd32
8 years ago

#20358 was marked as a duplicate.

#21 follow-up: @puggan
8 years ago

+1 ReOpen?

#22 in reply to: ↑ 21 ; follow-up: @SergeyBiryukov
8 years ago

Replying to puggan:

+1 ReOpen?

Would be good to have answers to comment:15.

#23 in reply to: ↑ 22 @puggan
7 years ago

Replying to SergeyBiryukov:

Replying to puggan:

+1 ReOpen?

Would be good to have answers to comment:15.

If you define a max-width, and don't have the height-attribute, it keeps the ratio.
If you define a max-width, and have a height-attribute, it keeps the height.

https://www.dropbox.com/s/xk8uz0jbdkgwl32/Screen%20Shot%202018-06-28%20at%2015.17.30.png

 <style> img { max-width: 100%; } </style> <div style="width: 150px"> <p> Image with attributes </p> <img src="https://www.google.se/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png" height="92" width="272" /> <p> Image without attributes </p> <img src="https://www.google.se/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png" /> </div> 

#24 @divinenephron
7 years ago

I don't recall the details of exactly why I needed this, but comment:23 (maintain image aspect ratio) sounds like my original reason for opening the ticket.

Apologies for the strangeness of the original patch – I had never committed to an open source project when I wrote it and didn't know about these unwritten norms.

#25 @puggan
7 years ago

The bugfix that worked up to 5.0.5 stoped working at 5.1.1 :-(
https://github.com/SpiroAB/WordPress/commit/a56e91190e49befc6a37ac36e317121a0168d8a5

Maybe just an bad merge.

Any news about reopening this ticket?

#26 @spacedmonkey
2 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

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


2 years ago
#27

  • Keywords has-patch added

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


2 years ago

#29 @adamsilverstein
2 years ago

  • Owner set to adamsilverstein
  • Status changed from reopened to assigned

#30 follow-up: @adamsilverstein
6 months ago

@spacedmonkey your PR has some conflicts that need resolving, do you have time to get it updated?

#31 @adamsilverstein
6 months ago

  • Keywords 2nd-opinion added

Expose height and width attributes to 'wp_get_attachment_image_attributes' filter

Reviewing the PR I noticed that we already include the height and with attributes in the data passed to wp_get_attachment_image_attributes (since #58235).

@divinenephron Is this PR still needed, or can we close this ticket now as already resolved?

Last edited 3 months ago by johnbillion (previous) (diff)

#32 in reply to: ↑ 30 @adamsilverstein
6 months ago

Replying to adamsilverstein:

@spacedmonkey your PR has some conflicts that need resolving, do you have time to get it updated?

Actually not sure this is still needed, what do you think?

#33 @divinenephron
6 months ago

I’m not involved in writing WordPress plugins any more, so I’m happy for this ticket to be closed whenever you think it’s appropriate.

#34 @divinenephron
6 months ago

  • Keywords reporter-feedback removed

#35 @adamsilverstein
6 months ago

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

Thanks for responding @divinenephron.

Given the height and width are now passed in the filter attributes, I am going to close this ticket as resolved.

#37 @spacedmonkey
6 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Not sure why this ticket was closed, the height and width is not in the filter.

I have recreated the PR. Can you take a look @adamsilverstein.

#38 @adamsilverstein
6 months ago

@spacedmonkey Apologies for closing, I was mistaken and thought the width and height were already included. I double checked and you are right, they aren't currently included.

The PR looks good, except there is one failing test - can you take a look? We Probably shouldn't output the width/height at all if the values are empty.

#39 @adamsilverstein
6 months ago

  • Milestone set to 6.8.2

#40 @spacedmonkey
6 months ago

@adamsilverstein tests now passing.

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


5 months ago

#42 @audrasjb
5 months ago

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

Hello there, we reviewed this ticket during today's 6.8.2 bug scrub.
Is this ticket still on track on your side @adamsilverstein or do you want to pass it to @spacedmonkey?
Anyway the PR now passes the previously failing test and looks good for a final review before commit and backport to branch 6.8.

#43 @adamsilverstein
4 months ago

  • Keywords commit added; dev-feedback removed

Thanks for the ping @audrasjb - I will continue working on this and get it to commit.

Thanks for updating the PR @spacedmonkey! I'd like to add some unit tests to validate the changes then I can get this committed.

#44 @spacedmonkey
4 months ago

  • Keywords has-unit-tests added

@adamsilverstein @flixos90 @audrasjb

Unit tests added.

#45 @sandeepdahiya
4 months ago

  • Keywords has-test-info added

Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: Patch-8758.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Firefox 139.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Image Attributes Debugger 1.0
    • Test Reports 1.2.0

Actual Results

  1. Before the patch - for wp_get_attachment_image_attributes attributes passed to the filter: -
Image attributes passed to the filter: Array ( [src] => http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-200x300.jpg [class] => attachment-medium size-medium [alt] => [srcset] => http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-200x300.jpg 200w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-684x1024.jpg 684w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-768x1150.jpg 768w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-1025x1536.jpg 1025w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-1367x2048.jpg 1367w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-scaled.jpg 1709w [sizes] => (max-width: 200px) 100vw, 200px ) 
  1. After the patch - attributes passed to the filter :-
Image attributes passed to the filter: Array ( [src] => http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-200x300.jpg [class] => attachment-medium size-medium [alt] => [width] => 200 [height] => 300 [srcset] => http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-200x300.jpg 200w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-684x1024.jpg 684w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-768x1150.jpg 768w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-1025x1536.jpg 1025w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-1367x2048.jpg 1367w, http://localhost:8889/wp-content/uploads/2025/06/oscar-sutton-yihlaRCCvd4-unsplash-2-scaled.jpg 1709w [sizes] => (max-width: 200px) 100vw, 200px ) 

Unit test report: -

$ npm run test:php -- --group 14110 > WordPress@6.9.0 test:php > node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpunit --group 14110 Installing... Running as single site... To run multisite, use -c tests/phpunit/multisite.xml Not running ajax tests. To execute these, use --group ajax. Not running ms-files tests. To execute these, use --group ms-files. Not running external-http tests. To execute these, use --group external-http. PHPUnit 9.6.23 by Sebastian Bergmann and contributors. Warning: Your XML configuration validates against a deprecated schema. Suggestion: Migrate your XML configuration using "--migrate-configuration"! ... 3 / 3 (100%) Time: 00:07.222, Memory: 191.00 MB OK (3 tests, 8 assertions) 

#46 follow-up: @SirLouen
4 months ago

  • Keywords has-test-info removed

@sandeepdahiya has-test-info should be added not as the opposite of needs-testing but as the opposite of needs-test-info. Although many times test info can be inferred from reading the whole topic, having clear steps on how to test, some code, or some testing use case, is the best scenario to add this tag.

Test info, in this case, are the steps of the protocol you used to test this. In this case, for example, posting the code you used in Image Attributes Debugger (with a GB repo, or directly here)

#47 in reply to: ↑ 46 ; follow-up: @sandeepdahiya
4 months ago

Replying to SirLouen:

Test info, in this case, are the steps of the protocol you used to test this. In this case, for example, posting the code you used in Image Attributes Debugger (with a GB repo, or directly here)

I created a short custom plugin to test the attributes passed to the filter. Just replace the image_id (in third last line) in the shortcode with the actual image id and add the shortcode to a post/page to check the results.

<?php /** * Plugin Name: Image Attributes Debugger * Description: Prints the attributes passed into the wp_get_attachment_image_attributes filter in the browser. * Version: 1.0 * Author: Sandeep Dahiya */ add_filter( 'wp_get_attachment_image_attributes', 'my_debug_image_attributes', 10, 2 ); function my_debug_image_attributes( $attr, $attachment ) { // check if the user has the capability to manage options if ( current_user_can ( 'manage_options' ) && is_admin() === false ) { // print the attributes in a preformatted block echo '<pre style="background: #eee; padding: 10px; border: 1px solid #ccc; ">'; echo 'Image attributes passed to the filter: ' . "\n"; print_r($attr); echo '</pre>'; } return $attr; } add_shortcode( 'my_test_image', 'my_test_image_shortcode' ); function my_test_image_shortcode() { $image_id = 7; return wp_get_attachment_image( $image_id, 'medium' ); } 

#48 in reply to: ↑ 47 @SirLouen
4 months ago

  • Keywords has-test-info added

Replying to sandeepdahiya:

I created a short custom plugin to test the attributes passed to the filter. Just replace the image_id (in third last line) in the shortcode with the actual image id and add the shortcode to a post/page to check the results.

👍

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


4 months ago

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


4 months ago

#51 @zunaid321
4 months ago

  • Keywords needs-testing added

This ticket was brought up in a recent bug scrub.

Here’s the feedback received:

@adamsilverstein 's update regarding this ticket will be highly appreciated.

Also added the keyword: needs-testing since @SirLouen will throw in another test report soon. Others are welcome to submit their test reports as well.

#52 @adamsilverstein
4 months ago

Thanks @zunaid321 - I'll get this committed.

#53 @adamsilverstein
4 months ago

  • Keywords needs-testing removed

#54 @adamsilverstein
4 months ago

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

In 60415:

Media: expose height and width attributes to the 'wp_get_attachment_image_attributes' filter.

Include the image height and width in the attributes passed to the 'wp_get_attachment_image_attributes' filter. Developers can use this to adjust the width and height attributes returned from the 'wp_get_attachment_image_attributes' function.

Props divinenephron, nacin, Sam_a, wpsmith, anatolbroder, ericlewis, puggan, SergeyBiryukov, spacedmonkey, adamsilverstein, flixos90, sandeepdahiya, SirLouen.
Fixes #14110.

#55 @audrasjb
4 months ago

  • Keywords fixed-major dev-reviewed added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 6.8 branch backport.
This comment count as second committer sign-off.

#56 @audrasjb
4 months ago

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

In 60420:

Media: expose height and width attributes to the wp_get_attachment_image_attributes filter.

Include the image height and width in the attributes passed to the wp_get_attachment_image_attributes filter. Developers can use this to adjust the width and height attributes returned from the wp_get_attachment_image_attributes function.

Reviewed by audrasjb.
Merges [60415] to the 6.8 branch.
Props divinenephron, nacin, Sam_a, wpsmith, anatolbroder, ericlewis, puggan, SergeyBiryukov, spacedmonkey, adamsilverstein, flixos90, sandeepdahiya, SirLouen.
Fixes #14110.

Note: See TracTickets for help on using tickets.