Moves feature classes from BODY element to HTML element

Move feature classes to HTML element - this is significant
for anonymous as placing the classes on the body tag breaks
the browser's ability to parse the article concurrently
with the stylesheet download, because inline scripts
are spec'ed to be able to see document.styleSheets.

Changes:
* Feature classes are moved from BODY tag to HTML tag
* For now disable localStorage storage until we've worked out
the storage mechanism in core.

Bug: T321498
Change-Id: Id5afe2c60dc0067e7c74433eda5cd7858f54b0d7

bundlesize.config.json

@@ -5,7 +5,7 @@
 	},
 	{
 		"resourceModule": "skins.vector.styles",
-		"maxSize": "11.1 kB"
+		"maxSize": "11.2 kB"
 	},
 	{
 		"resourceModule": "skins.vector.legacy.js",

includes/Hooks.php

@@ -6,7 +6,6 @@ use Config;
 use IContextSource;
 use MediaWiki\Auth\Hook\LocalUserCreatedHook;
 use MediaWiki\Hook\MakeGlobalVariablesScriptHook;
-use MediaWiki\Hook\OutputPageBodyAttributesHook;
 use MediaWiki\Hook\RequestContextCreateSkinHook;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Preferences\Hook\GetPreferencesHook;
@@ -32,7 +31,6 @@ class Hooks implements
 	GetPreferencesHook,
 	LocalUserCreatedHook,
 	MakeGlobalVariablesScriptHook,
-	OutputPageBodyAttributesHook,
 	ResourceLoaderSiteModulePagesHook,
 	ResourceLoaderSiteStylesModulePagesHook,
 	RequestContextCreateSkinHook,
@@ -650,26 +648,6 @@ class Hooks implements
 		}
 	}
 
-	/**
-	 * Called when OutputPage::headElement is creating the body tag to allow skins
-	 * and extensions to add attributes they might need to the body of the page.
-	 *
-	 * @param OutputPage $out
-	 * @param Skin $sk
-	 * @param string[] &$bodyAttrs
-	 */
-	public function onOutputPageBodyAttributes( $out, $sk, &$bodyAttrs ): void {
-		$skinName = $out->getSkin()->getSkinName();
-		if ( !self::isVectorSkin( $skinName ) ) {
-			return;
-		}
-		$config = $sk->getConfig();
-
-		$featureManager = VectorServices::getFeatureManager();
-		$bodyAttrs['class'] .= ' ' . implode( ' ', $featureManager->getFeatureBodyClass() );
-		$bodyAttrs['class'] = trim( $bodyAttrs['class'] );
-	}
-
 	/**
 	 * Temporary RequestContextCreateSkin hook handler.
 	 * Switches to new Vector on certain pages.

includes/SkinVector22.php

@@ -185,6 +185,8 @@ class SkinVector22 extends SkinMustache {
 	 */
 	public function getHtmlElementAttributes() {
 		$original = parent::getHtmlElementAttributes();
+		$featureManager = VectorServices::getFeatureManager();
+		$original['class'] .= ' ' . implode( ' ', $featureManager->getFeatureBodyClass() );
 
 		if ( VectorServices::getFeatureManager()->isFeatureEnabled( Constants::FEATURE_STICKY_HEADER ) ) {
 			// T290518: Add scroll padding to root element when the sticky header is
@@ -200,6 +202,7 @@ class SkinVector22 extends SkinMustache {
 			// possibly others). It must instead be applied to the html tag.
 			$original['class'] = implode( ' ', [ $original['class'] ?? '', self::STICKY_HEADER_ENABLED_CLASS ] );
 		}
+		$original['class'] = trim( $original['class'] );
 
 		return $original;
 	}

resources/skins.vector.es6/features.js

@@ -4,25 +4,12 @@ let /** @type {MwApi} */ api;
 const debounce = require( /** @type {string} */ ( 'mediawiki.util' ) ).debounce;
 
 /**
- * Saves preference to user preferences and/or localStorage.
+ * Saves preference to user preferences if user is logged in
  *
  * @param {string} feature
  * @param {boolean} enabled
  */
 function save( feature, enabled ) {
-	let featuresJSON;
-	// @ts-ignore
-	const features = mw.storage.get( 'VectorFeatures' ) || '{}';
-
-	try {
-		featuresJSON = JSON.parse( features );
-	} catch ( e ) {
-		featuresJSON = {};
-	}
-	featuresJSON[ feature ] = enabled;
-	// @ts-ignore
-	mw.storage.set( 'VectorFeatures', JSON.stringify( featuresJSON ) );
-
 	if ( !mw.user.isAnon() ) {
 		debounce( function () {
 			api = api || new mw.Api();
@@ -57,12 +44,40 @@ function toggleBodyClasses( name, override ) {
 	}
 }
 
+/**
+ *
+ * @param {string} name feature name
+ * @param {boolean} [override] option to force enabled or disabled state.
+ *
+ * @return {boolean} The new feature state (false=disabled, true=enabled).
+ * @throws {Error} if unknown feature toggled.
+ */
+function toggleDocClasses( name, override ) {
+	const featureClassEnabled = `vector-feature-${name}-enabled`,
+		classList = document.documentElement.classList,
+		featureClassDisabled = `vector-feature-${name}-disabled`;
+
+	if ( classList.contains( featureClassDisabled ) || override === true ) {
+		classList.remove( featureClassDisabled );
+		classList.add( featureClassEnabled );
+		return true;
+	} else if ( classList.contains( featureClassEnabled ) || override === false ) {
+		classList.add( featureClassDisabled );
+		classList.remove( featureClassEnabled );
+		return false;
+	} else {
+		// Perhaps we are dealing with cached HTML ?
+		// FIXME: Can be removed and replaced with throw new Error when cache is clear.
+		return toggleBodyClasses( name, override );
+	}
+}
+
 /**
  * @param {string} name
  * @throws {Error} if unknown feature toggled.
  */
 function toggle( name ) {
-	const featureState = toggleBodyClasses( name );
+	const featureState = toggleDocClasses( name );
 	save( name, featureState );
 }
 
@@ -73,9 +88,10 @@ function toggle( name ) {
  * @return {boolean}
  */
 function isEnabled( name ) {
-	return document.body.classList.contains(
+	const className = `vector-feature-${name}-enabled`;
-		'vector-feature-' + name + '-enabled'
+	return document.documentElement.classList.contains( className ) ||
-	);
+		// FIXME: For cached HTML. Remove after one train cycle.
+		document.body.classList.contains( className );
 }
 
-module.exports = { isEnabled, toggle, toggleBodyClasses };
+module.exports = { isEnabled, toggle, toggleDocClasses };

resources/skins.vector.es6/limitedWidthToggle.less

@@ -21,11 +21,11 @@
 	}
 
 	//NOTE: enabled/disabled class on body.
-	&.vector-feature-limited-width-disabled .vector-limited-width-toggle:before {
+	.vector-feature-limited-width-disabled .vector-limited-width-toggle:before {
 		background-image: url( images/fullscreen-close.svg );
 	}
 
-	&.vector-feature-limited-width-enabled .vector-limited-width-toggle:before {
+	.vector-feature-limited-width-enabled .vector-limited-width-toggle:before {
 		background-image: url( images/fullscreen.svg );
 	}
 }

resources/skins.vector.es6/pinnableElement.js

@@ -31,14 +31,14 @@ function disablePinningAtBreakpoint( header, e ) {
 	// FIXME: Class toggling should be centralized instead of being
 	// handled here, in features.js and togglePinnableClasses().
 	if ( e.matches && savedPinnedState === true ) {
-		features.toggleBodyClasses( featureName, false );
+		features.toggleDocClasses( featureName, false );
 		header.classList.remove( PINNED_HEADER_CLASS );
 		header.classList.add( UNPINNED_HEADER_CLASS );
 		movePinnableElement( pinnableElementId, unpinnedContainerId );
 	}
 
 	if ( !e.matches && savedPinnedState === true ) {
-		features.toggleBodyClasses( featureName, true );
+		features.toggleDocClasses( featureName, true );
 		header.classList.add( PINNED_HEADER_CLASS );
 		header.classList.remove( UNPINNED_HEADER_CLASS );
 		movePinnableElement( pinnableElementId, pinnedContainerId );

resources/skins.vector.styles/components/Header.less

@@ -32,11 +32,15 @@
 
 	@media ( min-width: @min-width-desktop-wide ) {
 		// Ensure search box is aligned with content when search thumbnails or JS is off
+		// FIXME: Remove html:not( .client-js ) when cache has cleared.
 		.vector-feature-page-tools-disabled & .vector-search-box:not( .vector-search-box-auto-expand-width ),
+		.vector-feature-page-tools-disabled:not( .client-js ) & .vector-search-box,
 		html:not( .client-js ) .vector-feature-page-tools-disabled & .vector-search-box {
 			padding-left: @size-search-expand;
 		}
 
+		// FIXME: Remove .client-js .vector-feature-page-tools-enabled when cache has cleared.
+		.client-js.vector-feature-page-tools-enabled & .vector-search-box.vector-search-box-auto-expand-width,
 		.client-js .vector-feature-page-tools-enabled & .vector-search-box.vector-search-box-auto-expand-width {
 			// Ensure search box is aligned with content when it autoexpands (i.e. search thumbnails)
 			margin-left: -@size-search-expand;

resources/skins.vector.styles/components/TableOfContentsPinned.less

@@ -18,11 +18,15 @@
 			contain: paint;
 		}
 
+		// FIXME: `.vector-feature-page-tools-disabled.vector-toc-pinned` can be removed when cached HTML no longer an issue.
+		.vector-feature-page-tools-disabled .vector-toc-pinned &,
 		.vector-feature-page-tools-disabled.vector-toc-pinned & {
 			// Align the left edge of the TOC text with the main menu button icon.
 			margin-left: -27px;
 		}
 
+		// FIXME: `.vector-feature-page-tools-enabled.vector-toc-pinned` can be removed when cached HTML no longer an issue.
+		.vector-feature-page-tools-enabled .vector-toc-pinned &,
 		.vector-feature-page-tools-enabled.vector-toc-pinned & {
 			// Align the left edge of the TOC text with the page container
 			margin-left: -@spacing-subsection-toggle;
@@ -40,11 +44,15 @@
 			padding-top: 1.5em;
 		}
 
+		// FIXME: .vector-feature-page-tools-disabled.vector-toc-pinned can be removed when cached HTML no longer an issue.
+		.vector-feature-page-tools-disabled .vector-toc-pinned @{selector-main-menu-closed} ~ .mw-table-of-contents-container &,
 		.vector-feature-page-tools-disabled.vector-toc-pinned @{selector-main-menu-closed} ~ .mw-table-of-contents-container & {
 			// Needed to align TOC with bottom of title, 1.5em padding + 1.5em margin = 3em
 			margin-top: 1.5em;
 		}
 
+		// FIXME: .vector-feature-page-tools-enabled.vector-toc-pinned can be removed when cached HTML no longer an issue.
+		.vector-feature-page-tools-enabled.vector-feature-main-menu-pinned-disabled .vector-toc-pinned &,
 		.vector-feature-page-tools-enabled.vector-toc-pinned.vector-feature-main-menu-pinned-disabled & {
 			// Align TOC with bottom of title when main menu is not pinned but the TOC is
 			margin-top: @margin-top-pinned-toc;

resources/skins.vector.styles/layouts/grid.less

@@ -108,7 +108,10 @@
 
 // Horizontally center content when column start is empty (i.e. no pinned ToC or pinned main menu)
 .vector-feature-page-tools-disabled {
+
+	// FIXME: &.vector-toc-unpinned can be removed when cache is clear.
 	@{selector-sidebar-no-toc-sidebar-closed},
+	.vector-toc-unpinned @{selector-main-menu-closed},
 	&.vector-toc-unpinned @{selector-main-menu-closed} {
 		& ~ .mw-content-container {
 			grid-column: mainMenu / pageContent;
@@ -121,7 +124,9 @@
 
 // Horizontally center content when column start is empty (i.e. no pinned ToC or pinned main menu)
 .vector-feature-page-tools-enabled {
+	// FIXME: &.vector-toc-unpinned can be removed when cache is clear.
 	&.vector-feature-main-menu-pinned-disabled .vector-sidebar-container-no-toc ~ .mw-content-container,
+	&.vector-feature-main-menu-pinned-disabled .vector-toc-unpinned .mw-content-container,
 	&.vector-toc-unpinned.vector-feature-main-menu-pinned-disabled .mw-content-container {
 		grid-column: mainMenu / pageContent;
 		margin-left: auto;

skin.json

@@ -175,7 +175,6 @@
 		"GetPreferences": "VectorHooks",
 		"LocalUserCreated": "VectorHooks",
 		"MakeGlobalVariablesScript": "VectorHooks",
-		"OutputPageBodyAttributes": "VectorHooks",
 		"ResourceLoaderSiteModulePages": "VectorHooks",
 		"ResourceLoaderSiteStylesModulePages": "VectorHooks",
 		"SkinPageReadyConfig": "VectorHooks"