WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 5 months ago

#39004 accepted enhancement

Alt attributes should be searchable in media library

Reported by: joedolson Owned by: joedolson
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Media Keywords: has-patch has-unit-tests needs-testing
Focuses: accessibility, administration Cc:

Description

The alt attribute is intended to be the alternative replacement value for an image. As such, if you're managing an image library correctly, it would be entirely reasonable that most images would have an alt attribute but no caption or description. However, this means that your only searchable field is the image title.

Attachments (7)

39004.patch (1.0 KB) - added by rommelxcastro 5 years ago.
39004.1.patch (1.0 KB) - added by rommelxcastro 5 years ago.
fixes query
39004.2.patch (2.0 KB) - added by rommelxcastro 5 years ago.
function name refactor
39004.3.patch (5.6 KB) - added by Mista-Flo 8 months ago.
3a2a27274bcb10c60e48e9abe9090507.gif (4.9 MB) - added by audrasjb 8 months ago.
39004.3.patch
Capture d’écran 2021-01-23 à 22.09.53.png (296.5 KB) - added by audrasjb 8 months ago.
13K+ images
348d997c73aca8cbfa5625b64104d032.gif (3.7 MB) - added by audrasjb 8 months ago.

Change History (50)

#1 @joedolson
5 years ago

See #22744 as a model for doing this.

#2 @joedolson
5 years ago

  • Owner set to joedolson
  • Status changed from new to accepted

#3 @joedolson
5 years ago

  • Version set to 3.0

Setting version to 3.0; but this has probably been the case from the beginning. I'm just copycatting @johnbillion. ;)

#4 @joedolson
5 years ago

  • Milestone changed from Awaiting Review to 4.8

@rommelxcastro
5 years ago

#5 @rommelxcastro
5 years ago

  • Keywords needs-testing added; needs-patch removed

First patch added, I will add another one since probably the function name needs to be renamed to clarify it doesn't just searches by title

@rommelxcastro
5 years ago

fixes query

@rommelxcastro
5 years ago

function name refactor

#6 @rommelxcastro
5 years ago

  • Keywords 2nd-opinion added

39004.2.patch has the same query as 39004.1.patch, but changes the function name to meta to reflects it searches by meta.

#7 @rommelxcastro
5 years ago

  • Keywords needs-unit-tests added

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


5 years ago

#9 @joemcgill
5 years ago

Take note of #39092. We will probably want to do something similar for this.

#10 @joemcgill
5 years ago

  • Keywords dev-feedback 2nd-opinion removed

Related: #39358. We'll need to be aware of query performance concerns introduced in [38625] / #22744 before finalizing the approach here.

Setting aside performance concerns for the moment, 39004.2.patch looks like a good initial approach. Thanks @rommelxcastro. We may need to handle some backwards compatibility for anyone who has decided to do something like: remove_filter( 'posts_clauses', '_filter_query_attachment_filenames' ); or if someone is calling that function directly to apply the filter to other query cases.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


4 years ago

#13 @joemcgill
4 years ago

  • Milestone changed from 4.8 to Future Release

This, unfortunately, will have to wait until we have a better answer for the query performance concerns brought up in #39358.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 years ago

#15 @afercia
4 years ago

@joemcgill willing to own this? :)

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


10 months ago

#18 @antpb
10 months ago

  • Milestone changed from Future Release to 5.7

This was mentioned as a good focus for Media in 5.7 but the ticket is in need of an owner and actionable next steps. We'll be reviewing it in the upcoming Media component meetings as we work toward the release of 5.7.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


9 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


9 months ago

#21 @joedolson
9 months ago

  • Keywords 2nd-opinion added

The frustrating thing about this ticket is that both the description and caption fields are easily searched, because they're part of the post table; but the alt attribute is not.

There are some hacky methods I can think of to handle this; e.g., do a construction where the alt attribute field is appended to post_content so that content is available for searches, then removed by output functions. This would give a performant method of searching this value, but feels a little dirty. That said, using the post_content as a data store for searchability has some pseudo-precedent with how Gutenberg structures data, so it's not totally out of line.

It would definitely work, but I feel a little dirty proposing it. Would appreciate a second opinion on the idea.

#22 @ricjcs
8 months ago

Solving this problem would be very useful.

As it is not possible to put the images in folders or categories, I add "tags" in the file name or in the alt attribute to facilitate the organization and search. But currently searching for the file name doesn't work either: #52336
I know there are plugins to organize by categories, but I'm a wordpress.com customer without access to plugins.

Last edited 8 months ago by ricjcs (previous) (diff)

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


8 months ago

#24 @Mista-Flo
8 months ago

  • Keywords has-patch has-unit-tests added; needs-unit-tests removed

Hey I have refreshed the patch and added unit tests, it seems to work, what do you think?

@Mista-Flo
8 months ago

#25 @audrasjb
8 months ago

  • Keywords needs-testing 2nd-opinion removed

I tested 39004.3.patch and it works fine.
See the results of my testing below. I uploaded an image without any reference to the term "vercors" and added this as the image alt attribute. The search form retrieved it.

I personally think this one is good to go.

@audrasjb
8 months ago

39004.3.patch

#26 @joedolson
8 months ago

  • Keywords needs-testing added

The problem with this methodology is performance; adding meta queries into the media library is expected to create significant problems in medium to large libraries, and those problems are likely to arise much earlier than expected.

This needs testing that demonstrates that there are no performance concerns before it's viable.

#27 @audrasjb
8 months ago

Makes sense @joedolson. Therefore, I tested the patch on a staging website of a client with 13.000+ high def images, and on a pre-production server, not locally. I couldn't see any performance difference from my initial testing (on a very small media library: less than 50 images).

I assume you want some tests on larger media libraries, so I won't remove the needs-testing workflow keyword, but as far as I can tell, the patch looks good to me IRL.

#28 @joedolson
8 months ago

We'll discuss it in the media meeting on Thursday. I'm definitely game to commit this if it seems plausible that the performance hit won't be overwhelming, and we can always revert it during RC if it proves to be a problem.

#29 @audrasjb
8 months ago

Sounds great to me 👍

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


8 months ago

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


8 months ago

#32 @adamsilverstein
8 months ago

Thanks for the testing with 13k images @audrasjb - I suspect the performance issues may be more noticeable as an end user when we hit libraries with 100k or more images.

While I'm sure that is rare, we still need to consider those cases. Perhaps we can use a function similar to wp_is_large_network that will disable this alt query for large media libraries (and is filterable so developers can adjust the threshold/behavior).

#33 @ocean90
8 months ago

As mentioned above, the performance issue is already being discussed in #39358. And based on #52336 the filter is not even used on wordpress.com. So the performance issue is real.

I think the question is, since the query is already there, does the additional OR clause make it any worse than it already is? How does the result of an EXPLAIN statement look before and after?

#34 @adamsilverstein
8 months ago

Ah yes, thanks @ocean90.

This comment from @dd32 does a great job of summarizing why queries with post meta are slow on large sites and what we might do about it: https://core.trac.wordpress.org/ticket/39358#comment:7

@joemcgill - Did we ever consider the idea of adding an "Advanced" checkbox next to the search input? This could be unchecked by default (sticky per user), and to get meta searching you would have to check it. Huge sites like wp.com might still disable it entirely, but for most users I feel like this would be a nice enhancement to have and having meta search be optional and disabled by default should address the performance concerns.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


8 months ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


8 months ago

#37 @joedolson
8 months ago

Given that the performance issues with meta searching are pretty much something we're stuck with, I'd be pretty happy with something that made it available as an option.

I'm not sure that 'Advanced' would be a suitable text for a checkbox, by itself, but an 'Advanced' checkbox that toggles a panel where you can select which meta fields to search could be useful.

This could be filterable, so that hypothetically any meta field could be searchable, which I can imagine as useful for some plugins.

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.


8 months ago

#39 @paaljoachim
8 months ago

Testing

Currently:
Testing a local site running Twenty Twenty One.
Adding "Alternative Text" text to some images.
Example adding the word screenshot to one of the images.
Writing the word screenshot into the search box.
Clicking enter on my keyboard (should be a button here) no images showed up in the search.

Testing the patch added by Mista-Flo 10 days ago.
Opening http://localhost:8889/
Adding a new image to the Media Library.
Giving it the "Alternative Text" word of screenshot.
Searching for the word screenshot.
The image shows up in the media library.


I clicked the "Add New" button.
Added a bunch of images, but I did not see them get uploaded. There was no reaction.
I suddenly noticed that the word screenshot was still located in the Search box field.
I removed the word and noticed that I had uploaded a lot of images.

The Search field should be automatically blanked out when clicking elsewhere. Here I test one thing, that now works and notice two things that would be nice to get fixed..:)

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


8 months ago

#41 @antpb
8 months ago

  • Milestone changed from 5.7 to 5.8

Hello! With 5.7 Beta 1 happening today, we'll need to move this into the next milestone as there are still some UI considerations to make here. It's been recommended we change the UI to make the search for alt attributes an advanced search option. How "Advanced Search" looks still needs to be determined. Before marking Needs Design maybe we can propose a way that would look. I'm still unclear on what that option would look like.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


5 months ago

#43 @ryokuhi
5 months ago

  • Milestone changed from 5.8 to Future Release

This ticket was discussed today during the accessibility team's bug-scrub.
We agreed with @joedolson to move this ticket to Future release, given that some architectural work will be needed to figure out search options to work with.

Note: See TracTickets for help on using tickets.