Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#36848 closed defect (bug) (fixed)

Theme installer: avoid to announce the search results too many times

Reported by: afercia's profile afercia Owned by: ocean90's profile ocean90
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-screenshots has-patch
Focuses: ui, accessibility, javascript Cc:

Description

Noticed two cases where the search results message is unnecessarily dispatched to the aria-live region.

1 Initial render of the view
When the view is initially rendered, looks like announceSearchResults() runs both on render and on the query:success event.

2 When using the arrow keys on the search input field
When doing a search in the Themes Installer and after the results are displayed users might want to select or edit the search terms in the input field to refine their search. To do this they will probably use the arrow keys (or the Home/End keys, etc.).

As soon as an arrow key is pressed, the search results message will be dispatched again to the aria-live region.

Investigated a bit and looks like this happens because there's nothing to check which keys get pressed or to check if the search terms have changed.

At the very least, I'd say there should be a check on the search terms, also for consistency with what happens in the Installed Themes screen.

TL;DR

While in the Installed Themes screen themes.view.Search.doSearch uses themes.Collection.doSearch which checks the current search input field value against the previous search and returns early if they're equal...
in the Themes Installer instead, themes.view.InstallerSearch does extend themes.view.Search but then implements its own search and doSearch functions which miss to check the search input field value against the previous search.

Thus, while previous search results are actually cached and a new API call is prevented, a query:success custom event is triggered anyways and on this event other things run again including:

  • the count text update
  • more importantly, announceSearchResults() runs again

Tried to catch this in the screenshot below, to make it easier to reproduce. Just run a search and then move the cursor in the input field using the arrow keys:

https://cldup.com/bWLPriMY_c.png

Attachments (1)

36848.patch (1.2 KB) - added by afercia 9 years ago.

Download all attachments as: .zip

Change History (5)

@afercia
9 years ago

#1 @afercia
9 years ago

  • Keywords has-patch added

#2 @afercia
9 years ago

Note: it's better to test this with Firefox and NVDA or IE and JAWS because of #36853.

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


9 years ago

#4 @ocean90
8 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 37967:

Themes: Avoid announcing the theme search results too many times.

Props afercia.
Fixes #36848.

Note: See TracTickets for help on using tickets.