Image Block: Prepend HTTP to links when missing protocol #71027
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @porg. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Thanks, @shrivastavanolo!
Left a couple of notes regarding e2e tests, but the general update looks good.
| imageBlock.locator( 'data-testid=form-file-upload-input' ) | ||
| ); | ||
|
|
||
| await page.getByLabel( 'Block tools' ).getByLabel( 'Link' ).click(); |
There was a problem hiding this comment.
| await page.getByLabel( 'Block tools' ).getByLabel( 'Link' ).click(); | |
| await editor.clickBlockToolbarButton( 'Link' ); |
There's a helper for this.
| // This form lacks distinguishing qualities other than the | ||
| // class name, so we use page.locator() instead of page.getByRole() | ||
| const form = page.locator( '.block-editor-url-popover__link-editor' ); |
There was a problem hiding this comment.
Let's target URL compobox instead, since we can reuse it.
page.getByRole( 'combobox', { name: 'URL' } );There was a problem hiding this comment.
Hey @Mamaduka Thanks for the review!
Here, I scoped both the URL input and the Apply button within the popover container (.block-editor-url-popover__link-editor) since the Apply button was clashing elsewhere on the page. Keeping it scoped also made sense for the URL input to keep things consistent and avoid any selector issues.
| const url = 'example.com'; | ||
|
|
||
| await expect( form.getByLabel( 'URL' ) ).toBeFocused(); | ||
| await form.getByLabel( 'URL' ).fill( url ); |
There was a problem hiding this comment.
- The
urlvariable is only used once. We can just inline the initial URL. - The
locator.fillensures that the field receives focus before filling it. There's no need for thetoBeFocusedassertion. See details section: https://playwright.dev/docs/api/class-locator#locator-fill. - Avoid using
getByLabelwhen possible.
|
I argued that under contemporary web practices and SEO that: Yet according to your summary your PR seems to:
HTTPS should be the default. Could we agree on that? |
What?
Closes #35910
Prepending
http://to a link without protocol ( for eg. 'www.example.com' ) added to the image block for correct linking.Why?
When a valid link without protocol ( for eg. 'www.example.com' ) is added to an image in Gutenberg, the WordPress site's base URL is incorrectly appended to the link, resulting in a 404 error. This behavior is inconsistent with links added in paragraph blocks, which link correctly.
Similar to #47311 which uses this methodology for the button block
How?
Modifies
image-url-input-ui.jsusingprependHttp()from@wordpress/url.Additionally, a new test case has been added to
image.spec.js.Testing Instructions
http://orhttps://using the block toolbar