WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 months ago

Last modified 3 months ago

#39041 closed defect (bug) (fixed)

Installed themes search displays theme author names when the search term is a space

Reported by: afercia Owned by: adamsilverstein
Milestone: 5.3 Priority: normal
Severity: normal Version: 3.9
Component: Themes Keywords: has-screenshots good-first-bug has-patch
Focuses: javascript Cc:

Description

Go in the themes screen and insert a single space in the search field: the theme author names will appear in the theme thumbnails:

https://cldup.com/ZClQwM6qwX.jpg

Theme author names are intended to appear just when the search term matches the author name. Side note: wondering how many users are aware of this "hidden" feature.

Looking a bit into theme.js, turns out that thanks to this replacement:

term = term.replace( / /g, ')(?=.*' );

the regular expression
match = new RegExp( '^(?=.*' + term + ').+', 'i' );
becomes
match = new RegExp( '^(?=.*)(?=.*).+', 'i' );

so it matches any author name and it is longer than 2 characters:

if ( match.test( data.get( 'author' ) ) && term.length > 2 ) {
	data.set( 'displayAuthor', true );
}

Without changing the regex, maybe the best option would be using jQuery.trim( term ) (because IE 8 is still supported) and then if the term is empty, just return early. Also, I'd like to quote @azaozz and recommend that this kind of searches should fire after a minimum number of typed characters, see: https://core.trac.wordpress.org/ticket/37233#comment:21

ideally it should trigger after 2 ASCII chars or one high UTF-8 char. We can standardize this for all similar cases in core, there are at least 6-7 other places.

Attachments (1)

39041.diff (477 bytes) - added by hesyifei 6 months ago.

Download all attachments as: .zip

Change History (7)

#1 @desrosj
6 months ago

  • Keywords good-first-bug needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version set to 3.9

Just tested this out on the latest version (5.1.1 as of this post) and this bug is still happening.

Worth noting that IE 8 is no longer supported (only IE11 and newer)., so that is no longer a consideration when solving.

@hesyifei
6 months ago

#2 @hesyifei
6 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

I think we could .trim() the term before finding results. .trim() is supported by all browsers (IE 9+).

#3 follow-up: @evalarumbe
3 months ago

Hey, I'm at WordCamp EU giving this a shot as my first contribution :) Will let you know how it goes!

#4 in reply to: ↑ 3 @evalarumbe
3 months ago

  • Resolution set to worksforme
  • Status changed from new to closed

Oh whoops, still finding my way around, I see a solution has been submitted :)

The solution submitted by @hesyifei is working for me on Mac High Sierra (10.13.6) on the following browsers:
Chrome 74.0.3729.169
Firefox 67.0.3
Safari 12.1.1

#5 @adamsilverstein
3 months ago

  • Owner set to adamsilverstein
  • Resolution changed from worksforme to fixed

In 45557:

Themes: improve search by trimming search string.

Fix an issue where searching installed themes for an empty string resulted in matching all themes.

Props afercia, desrosj, hesyifei, evalarumbe.
Fixes #39041.

#6 @pento
3 months ago

  • Keywords needs-testing removed
  • Milestone changed from Future Release to 5.3
Note: See TracTickets for help on using tickets.