Skip to content

Commit

Permalink
Improve (and relax) search vs direct URL entry detection in Link Cont…
Browse files Browse the repository at this point in the history
…rol (#51011)

* Restirct what matches as potentially being a “url”

* Remove direct entry results from coming up in entity search suggestions

* Make is-url-like stricter

* Add initial tests for isURLLike

* Improve code with tests and adding check for TLDs

* Simply implementation

* Fix tests

* Test for only showing URL result when searching for URL like

* Improve test criteria for URL-like in tests

* Augment tests for entity search
  • Loading branch information
getdave authored Jun 6, 2023
1 parent 3c45f35 commit 2c367ca
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 73 deletions.
43 changes: 40 additions & 3 deletions packages/block-editor/src/components/link-control/is-url-like.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { isURL } from '@wordpress/url';
import { getProtocol, isValidProtocol, isValidFragment } from '@wordpress/url';

/**
* Determines whether a given value could be a URL. Note this does not
Expand All @@ -15,6 +15,43 @@ import { isURL } from '@wordpress/url';
* @return {boolean} whether or not the value is potentially a URL.
*/
export default function isURLLike( val ) {
const isInternal = val?.startsWith( '#' );
return isURL( val ) || ( val && val.includes( 'www.' ) ) || isInternal;
const hasSpaces = val.includes( ' ' );

if ( hasSpaces ) {
return false;
}

const protocol = getProtocol( val );
const protocolIsValid = isValidProtocol( protocol );

const mayBeTLD = hasPossibleTLD( val );

const isWWW = val?.startsWith( 'www.' );

const isInternal = val?.startsWith( '#' ) && isValidFragment( val );

return protocolIsValid || isWWW || isInternal || mayBeTLD;
}

/**
* Checks if a given URL has a valid Top-Level Domain (TLD).
*
* @param {string} url - The URL to check.
* @param {number} maxLength - The maximum length of the TLD.
* @return {boolean} Returns true if the URL has a valid TLD, false otherwise.
*/
function hasPossibleTLD( url, maxLength = 6 ) {
// Clean the URL by removing anything after the first occurrence of "?" or "#".
const cleanedURL = url.split( /[?#]/ )[ 0 ];

// Regular expression explanation:
// - (?<=\S) : Positive lookbehind assertion to ensure there is at least one non-whitespace character before the TLD
// - \. : Matches a literal dot (.)
// - [a-zA-Z_]{2,maxLength} : Matches 2 to maxLength letters or underscores, representing the TLD
// - (?:\/|$) : Non-capturing group that matches either a forward slash (/) or the end of the string
const regex = new RegExp(
`(?<=\\S)\\.(?:[a-zA-Z_]{2,${ maxLength }})(?:\\/|$)`
);

return regex.test( cleanedURL );
}
125 changes: 82 additions & 43 deletions packages/block-editor/src/components/link-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,7 @@ describe( 'Basic rendering', () => {
within( resultsList ).getAllByRole( 'option' );

expect( searchResultElements ).toHaveLength(
// The fauxEntitySuggestions length plus the 'Press ENTER to add this link' button.
fauxEntitySuggestions.length + 1
fauxEntitySuggestions.length
);

// Step down into the search results, highlighting the first result item.
Expand Down Expand Up @@ -440,44 +439,87 @@ describe( 'Searching for a link', () => {
expect( screen.queryByRole( 'presentation' ) ).not.toBeInTheDocument();
} );

it( 'should display only search suggestions when current input value is not URL-like', async () => {
const user = userEvent.setup();
const searchTerm = 'Hello world';
const firstSuggestion = fauxEntitySuggestions[ 0 ];
it.each( [ 'With spaces', 'Uppercase', 'lowercase' ] )(
'should display only search suggestions (and not URL result type) when current input value (e.g. %s) is not URL-like',
async ( searchTerm ) => {
const user = userEvent.setup();
const firstSuggestion = fauxEntitySuggestions[ 0 ];

render( <LinkControl /> );
render( <LinkControl /> );

// Search Input UI.
const searchInput = screen.getByRole( 'combobox', { name: 'URL' } );
// Search Input UI.
const searchInput = screen.getByRole( 'combobox', { name: 'URL' } );

// Simulate searching for a term.
await user.type( searchInput, searchTerm );
// Simulate searching for a term.
await user.type( searchInput, searchTerm );

const searchResultElements = within(
await screen.findByRole( 'listbox', {
name: /Search results for.*/,
} )
).getAllByRole( 'option' );
const searchResultElements = within(
await screen.findByRole( 'listbox', {
name: /Search results for.*/,
} )
).getAllByRole( 'option' );

expect( searchResultElements ).toHaveLength(
fauxEntitySuggestions.length
);
expect( searchResultElements ).toHaveLength(
fauxEntitySuggestions.length
);

expect( searchInput ).toHaveAttribute( 'aria-expanded', 'true' );
expect( searchInput ).toHaveAttribute( 'aria-expanded', 'true' );

// Check that a search suggestion shows up corresponding to the data.
expect( searchResultElements[ 0 ] ).toHaveTextContent(
firstSuggestion.title
);
expect( searchResultElements[ 0 ] ).toHaveTextContent(
firstSuggestion.type
);
// Check that a search suggestion shows up corresponding to the data.
expect( searchResultElements[ 0 ] ).toHaveTextContent(
firstSuggestion.title
);
expect( searchResultElements[ 0 ] ).toHaveTextContent(
firstSuggestion.type
);

// The fallback URL suggestion should not be shown when input is not URL-like.
expect(
searchResultElements[ searchResultElements.length - 1 ]
).not.toHaveTextContent( 'URL' );
} );
// The fallback URL suggestion should not be shown when input is not URL-like.
expect(
searchResultElements[ searchResultElements.length - 1 ]
).not.toHaveTextContent( 'URL' );
}
);

it.each( [
[ 'https://wordpress.org', 'URL' ],
[ 'http://wordpress.org', 'URL' ],
[ 'www.wordpress.org', 'URL' ],
[ 'wordpress.org', 'URL' ],
[ 'ftp://wordpress.org', 'URL' ],
[ 'mailto:[email protected]', 'mailto' ],
[ 'tel:123456789', 'tel' ],
[ '#internal', 'internal' ],
] )(
'should display only URL result when current input value is URL-like (e.g. %s)',
async ( searchTerm, type ) => {
const user = userEvent.setup();

render( <LinkControl /> );

// Search Input UI.
const searchInput = screen.getByRole( 'combobox', { name: 'URL' } );

// Simulate searching for a term.
await user.type( searchInput, searchTerm );

const searchResultElement = within(
await screen.findByRole( 'listbox', {
name: /Search results for.*/,
} )
).getByRole( 'option' );

expect( searchResultElement ).toBeInTheDocument();

// Should only be the `URL` suggestion.
expect( searchInput ).toHaveAttribute( 'aria-expanded', 'true' );

expect( searchResultElement ).toHaveTextContent( searchTerm );
expect( searchResultElement ).toHaveTextContent( type );
expect( searchResultElement ).toHaveTextContent(
'Press ENTER to add this link'
);
}
);

it( 'should trim search term', async () => {
const user = userEvent.setup();
Expand All @@ -504,8 +546,7 @@ describe( 'Searching for a link', () => {
.flat()
.filter( Boolean );

// Given we're mocking out the results we should always have 4 mark elements.
expect( searchResultTextHighlightElements ).toHaveLength( 4 );
expect( searchResultTextHighlightElements ).toHaveLength( 3 );

// Make sure there are no `mark` elements which contain anything other
// than the trimmed search term (ie: no whitespace).
Expand Down Expand Up @@ -565,16 +606,15 @@ describe( 'Searching for a link', () => {
const lastSearchResultItem =
searchResultElements[ searchResultElements.length - 1 ];

// We should see a search result for each of the expect search suggestions
// plus 1 additional one for the fallback URL suggestion.
// We should see a search result for each of the expect search suggestions.
expect( searchResultElements ).toHaveLength(
fauxEntitySuggestions.length + 1
fauxEntitySuggestions.length
);

// The last item should be a URL search suggestion.
expect( lastSearchResultItem ).toHaveTextContent( searchTerm );
expect( lastSearchResultItem ).toHaveTextContent( 'URL' );
expect( lastSearchResultItem ).toHaveTextContent(
// The URL search suggestion should not exist.
expect( lastSearchResultItem ).not.toHaveTextContent( searchTerm );
expect( lastSearchResultItem ).not.toHaveTextContent( 'URL' );
expect( lastSearchResultItem ).not.toHaveTextContent(
'Press ENTER to add this link'
);
}
Expand Down Expand Up @@ -964,8 +1004,7 @@ describe( 'Default search suggestions', () => {
} )
).getAllByRole( 'option' );

// It should match any url that's like ?p= and also include a URL option.
expect( searchResultElements ).toHaveLength( 5 );
expect( searchResultElements ).toHaveLength( 4 );

expect( searchInput ).toHaveAttribute( 'aria-expanded', 'true' );

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Internal dependencies
*/
import isURLLike from '../is-url-like';

describe( 'isURLLike', () => {
it.each( [ 'https://wordpress.org', 'http://wordpress.org' ] )(
'returns true for a string that starts with an http(s) protocol',
( testString ) => {
expect( isURLLike( testString ) ).toBe( true );
}
);

it.each( [
'hello world',
'https:// has spaces even though starts with protocol',
'www. wordpress . org',
] )(
'returns false for any string with spaces (e.g. "%s")',
( testString ) => {
expect( isURLLike( testString ) ).toBe( false );
}
);

it( 'returns false for a string without a protocol or a TLD', () => {
expect( isURLLike( 'somedirectentryhere' ) ).toBe( false );
} );

it( 'returns true for a string beginning with www.', () => {
expect( isURLLike( 'www.wordpress.org' ) ).toBe( true );
} );

it.each( [ 'mailto:[email protected]', 'tel:123456' ] )(
'returns true for common protocols',
( testString ) => {
expect( isURLLike( testString ) ).toBe( true );
}
);

it( 'returns true for internal anchor ("hash") links.', () => {
expect( isURLLike( '#someinternallink' ) ).toBe( true );
} );

// use .each to test multiple cases
it.each( [
[ true, 'http://example.com' ],
[ true, 'https://test.co.uk?query=param' ],
[ true, 'ftp://openai.ai?param=value#section' ],
[ true, 'example.com' ],
[ true, 'http://example.com?query=param#section' ],
[ true, 'https://test.co.uk/some/path' ],
[ true, 'ftp://openai.ai/some/path' ],
[ true, 'example.org/some/path' ],
[ true, 'example_test.tld' ],
[ true, 'example_test.com' ],
[ false, 'example' ],
[ false, '.com' ],
[ true, '_test.com' ],
[ true, 'http://example_test.com' ],
] )(
'returns %s when testing against string "%s" for a valid TLD',
( expected, testString ) => {
expect( isURLLike( testString ) ).toBe( expected );
}
);
} );
Original file line number Diff line number Diff line change
Expand Up @@ -51,46 +51,23 @@ const handleEntitySearch = async (
val,
suggestionsQuery,
fetchSearchSuggestions,
directEntryHandler,
withCreateSuggestion,
withURLSuggestion,
pageOnFront
) => {
const { isInitialSuggestions } = suggestionsQuery;
let resultsIncludeFrontPage = false;

let results = await Promise.all( [
fetchSearchSuggestions( val, suggestionsQuery ),
directEntryHandler( val ),
] );
const results = await fetchSearchSuggestions( val, suggestionsQuery );

// Identify front page and update type to match.
results[ 0 ] = results[ 0 ].map( ( result ) => {
results.map( ( result ) => {
if ( Number( result.id ) === pageOnFront ) {
resultsIncludeFrontPage = true;
result.isFrontPage = true;
return result;
}

return result;
} );

const couldBeURL = ! val.includes( ' ' );

// If it's potentially a URL search then concat on a URL search suggestion
// just for good measure. That way once the actual results run out we always
// have a URL option to fallback on.
if (
! resultsIncludeFrontPage &&
couldBeURL &&
withURLSuggestion &&
! isInitialSuggestions
) {
results = results[ 0 ].concat( results[ 1 ] );
} else {
results = results[ 0 ];
}

// If displaying initial suggestions just return plain results.
if ( isInitialSuggestions ) {
return results;
Expand Down Expand Up @@ -150,12 +127,18 @@ export default function useSearchHandler(
val,
{ ...suggestionsQuery, isInitialSuggestions },
fetchSearchSuggestions,
directEntryHandler,
withCreateSuggestion,
withURLSuggestion,
pageOnFront
);
},
[ directEntryHandler, fetchSearchSuggestions, withCreateSuggestion ]
[
directEntryHandler,
fetchSearchSuggestions,
pageOnFront,
suggestionsQuery,
withCreateSuggestion,
withURLSuggestion,
]
);
}

1 comment on commit 2c367ca

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 2c367ca.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5187960221
📝 Reported issues:

Please sign in to comment.