-
Notifications
You must be signed in to change notification settings - Fork 4.7k
LinkControl: Prepend https:// to URLs on direct entry #75379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { URLInput } from '../'; | |||||||
| import LinkControlSearchResults from './search-results'; | ||||||||
| import { CREATE_TYPE } from './constants'; | ||||||||
| import useSearchHandler from './use-search-handler'; | ||||||||
| import { normalizeDirectEntryURL } from './use-search-handler'; | ||||||||
|
Comment on lines
14
to
+15
|
||||||||
| import useSearchHandler from './use-search-handler'; | |
| import { normalizeDirectEntryURL } from './use-search-handler'; | |
| import useSearchHandler, { normalizeDirectEntryURL } from './use-search-handler'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2981,6 +2981,190 @@ describe( 'Entity handling', () => { | |
| ); | ||
| } ); | ||
|
|
||
| describe( 'Direct entry URL normalization', () => { | ||
| it( 'should prepend https:// to plain domain names when pressing Enter', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, 'wordpress.org' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: 'https://wordpress.org', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should prepend https:// to domain names with www prefix when pressing Enter', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, 'www.wordpress.org' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: 'https://www.wordpress.org', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should NOT prepend https:// to mailto: links', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, 'mailto:test@example.com' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: 'mailto:test@example.com', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should NOT prepend https:// to tel: links', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, 'tel:123456789' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: 'tel:123456789', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should NOT prepend https:// to hash/anchor links', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, '#section-anchor' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: '#section-anchor', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should NOT prepend https:// to relative paths', async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think all these "Should not prepend https://" tests should get refactored to one looping test. |
||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, '/relative/path' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: '/relative/path', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should NOT double-prepend https:// to URLs that already have protocol', async () => { | ||
| const user = userEvent.setup(); | ||
| const onChange = jest.fn(); | ||
|
|
||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ onChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox', { | ||
| name: 'Search or type URL', | ||
| } ); | ||
|
|
||
| await user.type( searchInput, 'https://already-has-protocol.com' ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| expect( onChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: 'https://already-has-protocol.com', | ||
| } ) | ||
| ); | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'Accessibility association for entity links', () => { | ||
| it( 'should associate unlink button with help text via aria-describedby', () => { | ||
| const entityLink = { | ||
|
|
@@ -3298,53 +3482,60 @@ describe( 'URL validation', () => { | |
| { | ||
| description: 'valid URLs with protocol', | ||
| url: 'https://wordpress.org', | ||
| expectedUrl: 'https://wordpress.org', | ||
| searchPattern: /https:\/\/wordpress\.org/, | ||
| }, | ||
| { | ||
| description: 'valid URLs without protocol (without http://)', | ||
| url: 'www.wordpress.org', | ||
| expectedUrl: 'https://www.wordpress.org', | ||
| searchPattern: /www\.wordpress\.org/, | ||
| }, | ||
| { | ||
| description: 'hash links (internal anchor links)', | ||
| url: '#section', | ||
| expectedUrl: '#section', | ||
| searchPattern: /#section/, | ||
| }, | ||
| { | ||
| description: 'relative paths (URLs starting with /)', | ||
| url: '/handbook', | ||
| expectedUrl: '/handbook', | ||
| searchPattern: /\/handbook/, | ||
| }, | ||
| ] )( 'should accept $description', async ( { url, searchPattern } ) => { | ||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ mockOnChange } | ||
| /> | ||
| ); | ||
| ] )( | ||
| 'should accept $description', | ||
| async ( { url, expectedUrl, searchPattern } ) => { | ||
| render( | ||
| <LinkControl | ||
| value={ { url: '' } } | ||
| forceIsEditingLink | ||
| onChange={ mockOnChange } | ||
| /> | ||
| ); | ||
|
|
||
| const searchInput = screen.getByRole( 'combobox' ); | ||
| await user.type( searchInput, url ); | ||
| const searchInput = screen.getByRole( 'combobox' ); | ||
| await user.type( searchInput, url ); | ||
|
|
||
| // Wait for suggestion to appear and become stable | ||
| await screen.findByRole( 'option', { | ||
| name: searchPattern, | ||
| } ); | ||
| // Wait for suggestion to appear and become stable | ||
| await screen.findByRole( 'option', { | ||
| name: searchPattern, | ||
| } ); | ||
|
|
||
| triggerEnter( searchInput ); | ||
| triggerEnter( searchInput ); | ||
|
|
||
| // No validation error - should succeed | ||
| await waitFor( () => { | ||
| expect( mockOnChange ).toHaveBeenCalled(); | ||
| } ); | ||
| // No validation error - should succeed | ||
| await waitFor( () => { | ||
| expect( mockOnChange ).toHaveBeenCalled(); | ||
| } ); | ||
|
|
||
| expect( mockOnChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url, | ||
| } ) | ||
| ); | ||
| } ); | ||
| expect( mockOnChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: expectedUrl, | ||
| } ) | ||
| ); | ||
| } | ||
| ); | ||
|
|
||
| it( 'should skip validation for entity suggestions (posts, pages, categories)', async () => { | ||
| const entityLink = { | ||
|
|
@@ -3484,9 +3675,10 @@ describe( 'URL validation', () => { | |
| // a useful URL in practice. However, our validation philosophy is to | ||
| // trust the native URL constructor as the authoritative source - if the | ||
| // browser accepts it, we accept it. | ||
| // The URL is automatically prepended with https:// for consistency. | ||
| expect( mockOnChange ).toHaveBeenCalledWith( | ||
| expect.objectContaining( { | ||
| url: 'www.wordpress', | ||
| url: 'https://www.wordpress', | ||
| } ) | ||
| ); | ||
| } ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,37 @@ import { store as blockEditorStore } from '../../store'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const handleNoop = () => Promise.resolve( [] ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Normalizes a URL value by prepending https:// when appropriate. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Handles special protocols (mailto, tel), hash links, and relative paths. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * This is the single source of truth for URL normalization across LinkControl. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Used by both handleDirectEntry (suggestion flow) and direct submission (Enter key). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {string} val The URL value to normalize | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return {string} The normalized URL with protocol if needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function normalizeDirectEntryURL( val ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let type = URL_TYPE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const protocol = getProtocol( val ) || ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( protocol.includes( 'mailto' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type = MAILTO_TYPE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( protocol.includes( 'tel' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type = TEL_TYPE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( val?.startsWith( '#' ) ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type = INTERNAL_TYPE; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Only prepend https:// for standard URLs without valid protocols | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return type === URL_TYPE ? prependHTTPS( val ) : val; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+51
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Handles special protocols (mailto, tel), hash links, and relative paths. | |
| * | |
| * This is the single source of truth for URL normalization across LinkControl. | |
| * Used by both handleDirectEntry (suggestion flow) and direct submission (Enter key). | |
| * | |
| * @param {string} val The URL value to normalize | |
| * @return {string} The normalized URL with protocol if needed | |
| */ | |
| export function normalizeDirectEntryURL( val ) { | |
| let type = URL_TYPE; | |
| const protocol = getProtocol( val ) || ''; | |
| if ( protocol.includes( 'mailto' ) ) { | |
| type = MAILTO_TYPE; | |
| } | |
| if ( protocol.includes( 'tel' ) ) { | |
| type = TEL_TYPE; | |
| } | |
| if ( val?.startsWith( '#' ) ) { | |
| type = INTERNAL_TYPE; | |
| } | |
| // Only prepend https:// for standard URLs without valid protocols | |
| return type === URL_TYPE ? prependHTTPS( val ) : val; | |
| * Delegates to prependHTTPS, which preserves special protocols (mailto, tel), | |
| * hash links, and relative paths. | |
| * | |
| * @param {string} val The URL value to normalize | |
| * @return {string} The normalized URL with protocol if needed | |
| */ | |
| export function normalizeDirectEntryURL( val ) { | |
| return prependHTTPS( val ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I think this is increasing the level of complexity rather than finding a way to simplify it. I think validation and normalization can happen in one flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The README now claims URLs without a protocol are normalized when submitted “either by pressing Enter or clicking Apply”, but the Apply button submission path in
LinkControluses the rawcurrentUrlInputValue(only validation usesprependHTTPS). Either update the docs to only describe the Enter/suggestion behavior, or update the Apply/submit path to normalize (e.g., by applying the same normalization helper before callingonChange).