"Notebook scrolls to heading" test fails subtly

[krassowski]

Since recent fixes in ToC this test: "Table of Contents scrolling to heading › Notebook scrolls to heading" fails by positioning the heading slightly off:

Actual	Expected

[krassowski]

This appears unrelated to recent security fixes to ToC implementation and indeed was already failing before these were merged as seen in #15375.

[krassowski]

It is related to them after all. For context, this snapshot was updated in #15386 which changed the behaviour from aligning heading to bottom to aligning to the top (i.e. what users were expecting). However, the scrolling logic was not using the per-heading scrolling at the time (due to a bug in heading retrieval) and instead using cell scrolling fallback logic, see:

jupyterlab/packages/notebook/src/toc.ts

Lines 593 to 616 in 85445cc

	const el = headingToElement.get(heading);
	if (el) {
	if (this.scrollToTop) {
	el.scrollIntoView({ block: 'start' });
	} else {
	const widgetBox = widget.content.node.getBoundingClientRect();
	const elementBox = el.getBoundingClientRect();
	if (
	elementBox.top > widgetBox.bottom ||
	elementBox.bottom < widgetBox.top
	) {
	el.scrollIntoView({ block: 'center' });
	}
	}
	} else {
	console.debug('scrolling to heading: using fallback strategy');
	await widget.content.scrollToItem(
	widget.content.activeCellIndex,
	this.scrollToTop ? 'start' : undefined
	);
	}
	};

Once a fix for GHSA-4m77-cmpx-vjc4 was merged, it also fixed the bug which was leading the execution into the fallback branch.

The cell scrolling logic aligns to the cell so it gave us a nice padding above the cell. But there may be multiple headings in a cell.