Skip to content

Fix: Changing URL in link after changing text outside the popover resets it #75342

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.

Sign up for GitHub

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

Merged
Mamaduka merged 7 commits into from
Feb 11, 2026
+74 −23
Merged

Fix: Changing URL in link after changing text outside the popover resets it #75342

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
25d74a2
Fix: Changing URL in link after changing text outside the popover res…
Mamaduka Feb 6, 2026
a862b66
Remove useless e2e test
Mamaduka Feb 9, 2026
2faeb3e
Fix failing e2e tests
Mamaduka Feb 9, 2026
682d12e
Restore e2e test
Mamaduka Feb 10, 2026
50af4bb
Change inline comments and harden the check
Mamaduka Feb 11, 2026
9f8933e
Move logic into getFormatBoundary
Mamaduka Feb 11, 2026
e0ec85b
Update EMPTY_BOUNDARIES values
Mamaduka Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions packages/format-library/src/link/inline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ import { createLinkFormat, isValidHref, getFormatBoundary } from './utils';
import { link as settings } from './index';
import CSSClassesSettingComponent from './css-classes-setting';

// CSSClassesSettingComponent moved to its own file and imported above.

const FORMAT_BOUNDARY_EDGE_OFFSET = 1;
const LINK_SETTINGS = [
...LinkControl.DEFAULT_LINK_SETTINGS,
{
Expand Down Expand Up @@ -167,7 +166,23 @@ function InlineLinkUI( {

return;
} else if ( newText === richTextText ) {
newValue = applyFormat( value, linkFormat );
// Use explicit format boundaries rather than relying on
// the current selection which may be collapsed or
// misaligned after external value changes.
const boundary = isActive
? getFormatBoundary( value, {
type: 'core/link',
} )
: undefined;
newValue = applyFormat(
value,
linkFormat,
boundary?.start,
// Text *selection* always extends +1 beyond the edge of the format. Account for it.
boundary?.end !== undefined
? boundary.end + FORMAT_BOUNDARY_EDGE_OFFSET
Comment thread
Mamaduka marked this conversation as resolved.
Resolved
: undefined
);
} else {
// Scenario: Editing an existing link.

Expand Down Expand Up @@ -318,10 +333,8 @@ function getRichTextValueFromSelection( value, isActive ) {
} );

textStart = boundary.start;

// Text *selection* always extends +1 beyond the edge of the format.
// We account for that here.
textEnd = boundary.end + 1;
// Text *selection* always extends +1 beyond the edge of the format. Account for it.
textEnd = boundary.end + FORMAT_BOUNDARY_EDGE_OFFSET;

@ellatrix ellatrix Feb 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure what you mean here. If I have "text" selected, the selection is start: 0, end: 4?

@Mamaduka Mamaduka Feb 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If you have "text" selected and focus moves to the Link popover, selection collapses - start: 4, end: 4. The getFormatBoundary tries to expand start/end to the actual format, but because of a linked edge case it returns start: 0, end: 3 and we had to add the offset.

You can log something using console.log( { start: value.start, end: value.end }, boundary ); and run the current e2e test.

@ellatrix ellatrix Feb 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, you have a format at index 0, 1, 2, 3. There's no index 4 in the text? Also, if the selection resets after moving focus to the link popover, that sounds like a bug. Selection should not change

@Mamaduka Mamaduka Feb 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If you select the whole linked text with a double-click and change it while the popover is open. Selection will be collapsed, which is start: 4, end: 4. I assume it should start: 3, end: 3, and then applyFormat will handle it correctly, but as noted in the inline comments, the collapsed selection extends +1 beyond the edge of the format.

I understand it's an odd case, but it's definitely a bug somewhere.

P.S. It is easy to view the text changing flow via e2e test UI mode.

@ellatrix ellatrix Feb 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If selection starts with start:0 and end:4, and it's collapsed, then the result will either be start:0 end:0 or start:4 end:4. Not sure why it should be start:3 end:3. I don't know anything about getFormatBoundary, this utility is new to me, but if it's supposed to return a boundary then that util should be responsible for converting content length to a boundary and doing +1

@ellatrix ellatrix Feb 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or is it used elsewhere and is this fix also fixing a bug elsewhere?

@Mamaduka Mamaduka Feb 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it's internal utility for the Link format, which has a more complex editing flow compared to other formats.

@ellatrix ellatrix Feb 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So there was a bug wherever else that util is used?

@Mamaduka Mamaduka Feb 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was introduced in #33849. @getdave might have more context on the matter.

@getdave getdave Feb 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for wrangling this. I only noticed the ping today I'm afraid and I didn't get time to input. I appreciate the fix 🙇

}

// Get a RichTextValue containing the selected text content.
Expand Down
41 changes: 41 additions & 0 deletions test/e2e/specs/editor/blocks/links.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,47 @@ test.describe( 'Links', () => {
] );
} );

test( 'correctly updates the link when caret at outer edge of format boundary', async ( {
page,
editor,
LinkUtils,
} ) => {
await editor.insertBlock( {
name: 'core/paragraph',
attributes: {
content:
'<a href="https://wordpress.org/gutenberg">Gutenberg</a> is awesome',
},
} );

// Change the link text by typing to trigger a RichText value change.
await editor.canvas
.getByRole( 'link', { name: 'Gutenberg' } )
.dblclick();
await page.keyboard.type( 'Block Editor' );

const linkPopover = LinkUtils.getLinkPopover();
await expect( linkPopover ).toBeVisible();

// Edit only the URL.
await linkPopover.getByRole( 'button', { name: 'Edit' } ).click();
await linkPopover
.getByPlaceholder( 'Search or type URL' )
.fill( 'https://wordpress.org' );
await linkPopover.getByRole( 'button', { name: 'Apply' } ).click();

// The link should have the updated URL.
await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/paragraph',
attributes: {
content:
'<a href="https://wordpress.org">Block Editor</a> is awesome',
},
},
] );
} );

test( 'toggle state of advanced link settings is preserved across editing links', async ( {
page,
editor,
Expand Down
Loading