Popover is displayed after source element is destroyed

[llRandom]

Bug description:

I have an element that shows popover on hover with delay. If element is destroyed before tooltip is displayed popover shows at top-left page corner. Expected behavior - don't show popover if element has been destroyed

<button *ngIf="!hidden"
  ngbPopover="test action"
  container="body"
  triggers="mouseenter:mouseleave"
  [openDelay]="1000"
  (click)="hidden = true">
  test action
</button>

Click on button before tooltip is displayed.

Link to minimally-working StackBlitz that reproduces the issue:

https://stackblitz.com/edit/angular-cdwked

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 17.0.8

ng-bootstrap: 16.0.0

Bootstrap: 5.3.2

[ChristianRosenau]

Also happens with basic tooltips if you set a [openDelay]. This is a real bummer after upgrading to the latest version.

A quick look into the code in src/util/triggers.ts identifies this function as an error candidate. Seems that the timeout is not cleared after tooltip is destroyed

function withDelay(fn: () => void, delayMs: number) {
	clearTimeout(timeout);
	if (delayMs > 0) {
		timeout = setTimeout(fn, delayMs);
	} else {
		fn();
	}
}

[Nness]

The code is using _getPositionTargetElement to get source element, there are two issue with given code.

1. _getPositionTargetElement's outcome never checked. It just assume it exist.
2. _getPositionTargetElement has been called again in this._ngZone.runOutsideAngular. Even when JavaScript is single threaded, but we are unsure once passed into expression. whether it is delayed execution or not. Either the code should get reference from open() method, or grab again and check whether exist or not before execution.

Of course there might be better way of handling this, this is just my preference on how to handle code.

ng-bootstrap/src/tooltip/tooltip.ts

Lines 221 to 247 in b5d5feb

	this._getPositionTargetElement().setAttribute('aria-describedby', this._ngbTooltipWindowId);
	if (this.container === 'body') {
	this._document.body.appendChild(this._windowRef.location.nativeElement);
	}
	// We need to detect changes, because we don't know where .open() might be called from.
	// Ex. opening tooltip from one of lifecycle hooks that run after the CD
	// (say from ngAfterViewInit) will result in 'ExpressionHasChanged' exception
	this._windowRef.changeDetectorRef.detectChanges();
	// We need to mark for check, because tooltip won't work inside the OnPush component.
	// Ex. when we use expression like `{{ tooltip.isOpen() : 'opened' : 'closed' }}`
	// inside the template of an OnPush component and we change the tooltip from
	// open -> closed, the expression in question won't be updated unless we explicitly
	// mark the parent component to be checked.
	this._windowRef.changeDetectorRef.markForCheck();
	// Setting up popper and scheduling updates when zone is stable
	this._ngZone.runOutsideAngular(() => {
	this._positioning.createPopper({
	hostElement: this._getPositionTargetElement(),
	targetElement: this._windowRef!.location.nativeElement,
	placement: this.placement,
	baseClass: 'bs-tooltip',
	updatePopperOptions: (options) => this.popperOptions(addPopperOffset([0, 6])(options)),
	});

[ChristianRosenau]

I ended up with patching it by myself by adding the line
cleanupFns.push(() => clearTimeout(timeout));
to the end of the listenToTriggers() function in util/triggers.
This does it for me as the cleanupFns seem to be called after the tooltip reference is destroyed. When a open delay timeout is running it should be canceled this way. I don't knowing the rest of the code very well, but there should be no other side effects.

[Nness]

That certainly works.