From ef59d7b973f21fe683a748ef3f0e4fe3c1e168e7 Mon Sep 17 00:00:00 2001
From: jdlrobson <jdlrobson@gmail.com>
Date: Wed, 15 Dec 2021 09:51:44 -0800
Subject: [PATCH] Disable table of contents in article body

This will currently remove table of contents in article body
for legacy and modern skin.

To prevent us deploying this in current form, a check is
added in generateHTML

This requires an adjustment of OverridableConfigRequirement to
support requirements which do not vary on whether the user is
logged in or not.

Bug: T297610
Change-Id: I847a284229e498b3aa04c16ea3f84c360e735052
---
 .../OverridableConfigRequirement.php          |  8 ++++++-
 includes/ServiceWiring.php                    |  1 -
 includes/SkinVector.php                       | 22 +++++++++++++++++--
 skin.json                                     |  3 +--
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php b/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php
index 35aa6acfe..08fa7c3f2 100644
--- a/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php
+++ b/includes/FeatureManagement/Requirements/OverridableConfigRequirement.php
@@ -188,6 +188,10 @@ final class OverridableConfigRequirement implements Requirement {
 				'logged_in' => $thisConfig,
 				'logged_out' => $thisConfig,
 			];
+		} elseif ( array_key_exists( 'default', $thisConfig ) ) {
+			$thisConfig = [
+				'default' => $thisConfig['default'],
+			];
 		} else {
 			$thisConfig = [
 				'logged_in' => $thisConfig['logged_in'] ?? false,
@@ -196,6 +200,8 @@ final class OverridableConfigRequirement implements Requirement {
 		}
 
 		// Fallback to config.
-		return $thisConfig[ $this->user->isRegistered() ? 'logged_in' : 'logged_out' ];
+		return array_key_exists( 'default', $thisConfig ) ?
+			$thisConfig[ 'default' ] :
+			$thisConfig[ $this->user->isRegistered() ? 'logged_in' : 'logged_out' ];
 	}
 }
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index bdc26284a..cbc9a33d5 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -198,7 +198,6 @@ return [
 			Constants::FEATURE_TABLE_OF_CONTENTS,
 			[
 				Constants::REQUIREMENT_FULLY_INITIALISED,
-				Constants::REQUIREMENT_LATEST_SKIN_VERSION,
 				Constants::REQUIREMENT_TABLE_OF_CONTENTS
 			]
 		);
diff --git a/includes/SkinVector.php b/includes/SkinVector.php
index 8a2be3e7c..8e1aed7c1 100644
--- a/includes/SkinVector.php
+++ b/includes/SkinVector.php
@@ -110,6 +110,18 @@ class SkinVector extends SkinMustache {
 	 */
 	private const OPT_OUT_LINK_TRACKING_CODE = 'vctw1';
 
+	/**
+	 * Updates the constructor to conditionally disable table of contents in article
+	 * body.
+	 * Note, the constructor can only check feature flags that do not vary on whether the
+	 * user is logged in e.g. features with the 'default' key set.
+	 * @inheritDoc
+	 */
+	public function __construct( array $options ) {
+		$options['toc'] = !$this->isTableOfContentsVisibleInSidebar();
+		parent::__construct( $options );
+	}
+
 	/**
 	 * Whether or not the legacy version of the skin is being used.
 	 *
@@ -375,6 +387,12 @@ class SkinVector extends SkinMustache {
 	public function generateHTML() {
 		if ( $this->isLegacy() ) {
 			$this->options['template'] = 'skin-legacy';
+			if ( $this->isTableOfContentsVisibleInSidebar() ) {
+				throw new RuntimeException(
+					'The table of contents flag cannot safely be applied without ' .
+					'breaking legacy Vector. Please fix T291098 before applying.'
+				);
+			}
 		}
 		return parent::generateHTML();
 	}
@@ -466,7 +484,7 @@ class SkinVector extends SkinMustache {
 	 *
 	 * @return bool
 	 */
-	private function isTableOfContentsVisible(): bool {
+	private function isTableOfContentsVisibleInSidebar(): bool {
 		$featureManager = VectorServices::getFeatureManager();
 		return $featureManager->isFeatureEnabled( Constants::FEATURE_TABLE_OF_CONTENTS );
 	}
@@ -508,7 +526,7 @@ class SkinVector extends SkinMustache {
 			'is-language-in-content-top' => $this->isLanguagesInContentAt( 'top' ),
 			'is-language-in-content-bottom' => $this->isLanguagesInContentAt( 'bottom' ),
 
-			'is-vector-table-of-contents-visible' => $this->isTableOfContentsVisible(),
+			'is-vector-table-of-contents-visible' => $this->isTableOfContentsVisibleInSidebar(),
 
 			'data-search-box' => $this->getSearchData(
 				$parentData['data-search-box'],
diff --git a/skin.json b/skin.json
index 7e3762105..821b60510 100644
--- a/skin.json
+++ b/skin.json
@@ -423,8 +423,7 @@
 		},
 		"VectorTableOfContents": {
 			"value": {
-				"logged_in": false,
-				"logged_out": false
+				"default": false
 			},
 			"description": "@var When `VectorTableOfContents` is enabled, the sticky table of contents is shown."
 		}