Skip to content

Commit

Permalink
Merge pull request #16363 from ckeditor/cc/6219-update-race
Browse files Browse the repository at this point in the history
Fix (ui): Prevented editor error in a situation where tooltip was unpinned after it was already removed. This happened when "Unlink" button was pressed while the tooltip was shown.
  • Loading branch information
scofalik authored May 15, 2024
2 parents 94fe004 + a36ee84 commit d318c5a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
10 changes: 8 additions & 2 deletions packages/ckeditor5-ui/src/tooltipmanager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,13 @@ export default class TooltipManager extends DomEmitterMixin() {
* Hides the tooltip when the element is no longer visible in DOM or the tooltip text was removed.
*/
private _updateTooltipPosition() {
const tooltipData = getTooltipData( this._currentElementWithTooltip! );
// The tooltip might get removed by focus listener triggered by the same UI `update` event.
// See https://github.com/ckeditor/ckeditor5/pull/16363.
if ( !this._currentElementWithTooltip ) {
return;
}

const tooltipData = getTooltipData( this._currentElementWithTooltip );

// This could happen if the tooltip was attached somewhere in a contextual content toolbar and the toolbar
// disappeared (e.g. removed an image), or the tooltip text was removed.
Expand All @@ -478,7 +484,7 @@ export default class TooltipManager extends DomEmitterMixin() {
}

this.balloonPanelView.pin( {
target: this._currentElementWithTooltip!,
target: this._currentElementWithTooltip,
positions: TooltipManager.getPositioningFunctions( tooltipData.position )
} );
}
Expand Down
27 changes: 27 additions & 0 deletions packages/ckeditor5-ui/tests/tooltip/tooltipmanager.js
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,33 @@ describe( 'TooltipManager', () => {
sinon.assert.calledOnce( pinSpy );
sinon.assert.calledOnce( unpinSpy );
} );

it( 'should not crash when the tooltip gets removed on the same UI `update` event', () => {
utils.dispatchMouseEnter( elements.a );
utils.waitForTheTooltipToShow( clock );

sinon.assert.calledOnce( pinSpy );

editor.ui.update();
sinon.assert.calledTwice( pinSpy );

expect( editor.editing.view.document.isFocused ).to.be.false;

// Minimal case of unlinking with the button in the link balloon toolbar.
// See https://github.com/ckeditor/ckeditor5/pull/16363.
editor.ui.once( 'update', () => {
editor.editing.view.focus();
} );

// After removing a link from content, model changed so view and DOM got updated.
editor.ui.update();

utils.waitForTheTooltipToHide( clock );

editor.ui.update();

sinon.assert.calledTwice( pinSpy );
} );
} );

describe( '#defaultPositions', () => {
Expand Down

0 comments on commit d318c5a

Please sign in to comment.