Opened 13 years ago
Closed 5 months ago
#21221 closed enhancement (wontfix)
Image title and alt attribute content should be texturized.
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Priority: | normal | |
| Severity: | normal | Version: | 3.4.1 |
| Component: | Media | Keywords: | dev-feedback has-patch close |
| Focuses: | Cc: |
Description
gallery_shortcode() texturizes the caption shown underneath images in galleries.
For consistency, alt and title tags content should also be texturized.
This is also valuable for developers extending the gallery shortcode or output, such as with the WordPress.com (and Jetpack) Gallery Carousel feature, as it provides i18n'd texturization, for EG.
See attached patch, which:
- uses wptexturize() in wp_get_attachment_image() directly (/wp-includes/media.php), which makes it work with gallries, attachment pages, etc.
- also uses wptexturize() in get_image_tag() (/wp-includes/media.php), for consistency.
- uses wptexturize() in wp_get_attachment_link() (/wp-includespost-template.php), for consistency
Attachments (5)
Change History (26)
#2
follow-up:
↓ 4
@
12 years ago
- Focuses accessibility added
- Keywords reporter-feedback dev-feedback added; has-patch removed
Why is esc_attr() not enough already? What's the benefit of wp_texturize()?
#4
in reply to:
↑ 2
@
11 years ago
Replying to ericlewis:
Why is
esc_attr()not enough already? What's the benefit ofwp_texturize()?
these do different things entirely - esc_attr is security related, stripping scripts, etc. wptexturize transforms text - "quotes to smart quotes, apostrophes, dashes, ellipses, the trademark symbol, and the multiplication symbol. "
I think would be nice to see this improved typography in alt and title tags, although I'm a bit wary of potential pitfalls or unexpected results.
@brianhogg is working on an updated patch here at #wcto. The current patch no longer applies cleanly.
#5
@
11 years ago
- Keywords needs-unit-tests added; reporter-feedback dev-feedback removed
- Milestone changed from Awaiting Review to Future Release
This should have tests to show what is not happening now and what should happen when the patch is applied.
#6
follow-up:
↓ 7
@
11 years ago
I've added some initial unit tests. They are my first core tests so any feedback is appreciated. I am re-using some of the data providers already in the class and if that approach makes sense, I'll go back and add the rest.
#7
in reply to:
↑ 6
@
11 years ago
- Keywords dev-feedback added; needs-unit-tests removed
- Milestone changed from Future Release to 4.2
Looks good @welcher, I haven't seen that @dataProvider technique before, excellent.
I verified tests failed before patch and pass after...
In 21221.4.diff I refreshed @brianhogg's patch and combined with your tests, plus made some tiny code standards fixes (missing space near closing bracket of unit test functions).
Replying to welcher:
I've added some initial unit tests. They are my first core tests so any feedback is appreciated. I am re-using some of the data providers already in the class and if that approach makes sense, I'll go back and add the rest.
#9
@
11 years ago
21221.4.diff seems safe, and I have some big picture questions about it:
- Why does an alt attribute need to be texturized?
- Why use an HTML parser in an attribute context?
- Is it okay to not texturize title attributes when they contain angle braces?
#10
@
11 years ago
I don't have answers for the second two questions; but for the first, the problem was for internationalization. Without texturizing, multibyte characters in attributes weren't expressed correctly.
#11
@
11 years ago
wptexturize is an ASCII only search and replace algorithm. What does this have to do with multibyte characters?
#12
@
11 years ago
Never mind then! I thought that was @stephdau's original intent from my prior review of this ticket, but if it's not a valid reason, then I'm not sure. I've never looked at this ticket in significant depth.
This ticket was mentioned in Slack in #core by drew. View the logs.
11 years ago
#15
@
11 years ago
- Milestone changed from 4.2 to Future Release
Looks like the feedback from @miqrogroove hasn't yet been fully addressed. Pushing to a future release.
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
10 years ago
#18
@
10 years ago
Just did some testing with screen readers, and this doesn't have any bearing on accessibility. The characters are read correctly whether they're texturized or not. Removing accessibility focus.
#19
@
5 months ago
- Keywords 2nd-opinion added
Hey all,
I have done some digging here, and I am not sure what the benefit of making this change would be? As already pointed out by @joedolson there is no benefit to accessibility here, but I am also struggling to see the SEO and UI/UX benefits?
In regards to SEO, search engines parse the alt text to understand image content and are more concerned with the words and semantic meaning, as opposed the typographic correctness of the punctuation. There is absolutely no known SEO advantage to using curly/pretty quotes over straight quotes in alt or title tags.
In regards to the UI/UX benefits this may make the front end look prettier when a page fails to load and the alt tag has been texturized, but as we are actively trying to avoid images failing to load, then this wouldn't really show up that often. But to achieve this we may introduce more issues as HTML attributes are themselves enclosed in straight quotes (e.g., alt="..."). While modern browsers are incredibly resilient, programmatically inserting different types of quotes inside the attribute's value can introduce a layer of unnecessary complexity. But also the purpose of an alt text is to be a clean, functional data string.
It is generally considered best practice is to keep the content of attributes as simple and unambiguous as possible to ensure maximum compatibility across all platforms and user agents. Therefore I suggest we close this ticket, however it would be good to get a 2nd-opinion here. 😃
#20
@
5 months ago
- Keywords close added; 2nd-opinion removed
Looking back over this, I fail to see any real benefit to texturizing alt or title attribute values here. I think there is some risk to causing problems by doing this, as you observe. Globally changing the values of attributes on sites could easily lead to problems if escaping isn't done properly on the client end, for example.
I agree with closing this ticket.
#21
@
5 months ago
- Resolution set to wontfix
- Status changed from new to closed
I also agree to close.
Jetpack's Gallery Carousel uses data- attributes and added wptexturize() for title, description and caption.
If appropriate, individual sites or plugins similarly could use the wp_get_attachment_image_attributes hook to run the function on attributes in [gallery] shortcode images.
add_filter( 'wp_get_attachment_image_attributes', 'use_wptexturize_for_alt_attr' );
function use_wptexturize_for_alt_attr( $attr ) {
$attr['alt'] = wptexturize( $attr['alt'] );
return $attr;
}
Do not hard code the function, use a
apply_filters()instead. See #19550.