Opened 15 years ago
Last modified 2 weeks ago
#14110 reopened enhancement
Expose height and width attributes to 'wp_get_attachment_image_attributes' filter
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.8.2 | Priority: | normal |
Severity: | minor | Version: | 3.0 |
Component: | Media | Keywords: | has-patch 2nd-opinion |
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)
Change History (44)
#2
@
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.
#3
@
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
@
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:
↓ 8
↓ 9
@
14 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
@
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
@
13 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.
#8
in reply to:
↑ 5
@
13 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.
@
13 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
@
13 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. ;)
#10
@
13 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?
#11
@
13 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.
#13
@
12 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.
11 years ago
#15
@
11 years ago
- Keywords reporter-feedback added; has-patch 2nd-opinion removed
- Why were we setting the image width/height attributes to begin with?
- Why would you need to override this for responsive design?
max-width: 100%
(or something like it) doesn't suffice?
#16
@
11 years ago
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from new to closed
No reporter feedback in 4 months.
#23
in reply to:
↑ 22
@
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.
<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
@
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
@
6 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?
This ticket was mentioned in PR #4738 on WordPress/wordpress-develop by @spacedmonkey.
23 months ago
#27
- Keywords has-patch added
Trac ticket: https://core.trac.wordpress.org/ticket/14110
This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.
21 months ago
#30
follow-up:
↓ 32
@
3 weeks ago
@spacedmonkey your PR has some conflicts that need resolving, do you have time to get it updated?
#31
@
2 weeks 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 r58235).
@divinenephron Is this PR still needed, or can we close this ticket now as already resolved?
#32
in reply to:
↑ 30
@
2 weeks 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
@
2 weeks 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.
#35
@
2 weeks 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.
This ticket was mentioned in PR #8758 on WordPress/wordpress-develop by @spacedmonkey.
2 weeks ago
#36
Trac ticket: https://core.trac.wordpress.org/ticket/14110
#37
@
2 weeks 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
@
2 weeks 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.
Patch to give the 'wp_get_attachment_image_attributes' filter access to the heigh and width attribues.