The TOC will appear empty if unset( $parentData['data-toc'] ); was not called inside Skin::getTemplateData or OutputPage::isTOCEnabled returns false (see core:includes/skins/components/SkinComponentTableOfContents.php). The only way the latter is possible is if there's a bug in the parser: (OutputPage::addParserOutputMetadata has set it to false because $parserOutput->getTOCHTML() is falsey.

When the error shows itself to me, I see the <mw:tocplace> element in the page HTML. This gives me confidence the parser is working as expected so I don't think the parser is to blame here.

I believe the reason this is breaking is twofold

1. $this->getSkinName() === 'vector' in certain circumstances relating to the skin migration Clare and I are trying to complete. Note this will happen in the following circumstances:

This can happen when your config has a mismatch between vector and skin version

$wgDefaultSkin = 'vector';
$wgVectorDefaultSkinVersion = '2';
$wgVectorDefaultSkinVersionForExistingAccounts = '2';
$wgVectorDefaultSkinVersionForNewAccounts = '2';

OR you've got an account which has the old preferences set (check that by looking in your database)

2. isTableOfContentsVisibleInSidebar() will return false if REQUIREMENT_FULLY_INITIALISED hasn't been met. This will happen if ServiceWiring hasn't been setup. If you are noticing this error only when you are loading your MediaWiki instance for the first time, you'll hitting this. Note this won't be a problem when we complete the migration as we'll no longer need to use the User object to look up preferences relating to SkinVersion at a time when User lookup is not possible.

Both issues should be addressed by T301930. I suggest we wait for that to happen (goal is to merge that next week and deploy it the week after). If we're still seeing this issue in development after that patch is merged, I'd be surprised, but in that situation I suggest we add some logging to understand the issue better. Does that sound like a plan?

I've noticed that this line returns an empty array when the empty TOC shows for me:

$outputSections = $this->output->getSections();

This seems to always happen on the first page load after I toggle config related to the TOC (e.g. enabling $wgVectorTableOfContents or enabling $wgVectorWebABTestEnrollment with the TOC work I've been doing). Subsequent page loads show the full TOC. I've also removed all skin version config and my database shows the skin set to vector-2022 with no VectorSkinVersion record so there might be several issues happening here

Change 771685 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Never show an empty table of contents

https://gerrit.wikimedia.org/r/771685

This seems to always happen on the first page load after I toggle config related to the TOC

If you are toggling config, then yes, this will happen as you are using parser cache and the old version of parser cache won't have information on sections. Looks like there is indeed a bug in how we do this.

You can see this very clearly if you hardcode in the SkinVector22 constructor:

		$options['toc'] = true;

End result: table of contents visible in the article and also the incomplete one in the sidebar.

I've posted a patch as this looks like a regression.

When deploying/ running the A/B test we'll likely need to consider whether the cached HTML state is being shown or whether it's acceptable to show no table of contents on cached pages.

Change 771685 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Never show an empty table of contents

https://gerrit.wikimedia.org/r/771685

@ovasileva when we switch on the table of contents cached pages will display no table of contents at all.

However, I believe our A/B test should alleviate this issue, as the A/B test will ensure that both are present on the page. With that in mind, can we resolve this ticket?

In T303860#7798567, @Jdlrobson wrote:

@ovasileva when we switch on the table of contents cached pages will display no table of contents at all.

However, I believe our A/B test should alleviate this issue, as the A/B test will ensure that both are present on the page. With that in mind, can we resolve this ticket?

What would be our solution eventually for after the A/B test, when we decide to deploy the feature in full?

What would be our solution eventually for after the A/B test, when we decide to deploy the feature in full?

When we disable the A/B test presumably we will enable it everywhere. At this point caching will not be a concern because the A/B test has been running for a few weeks.