From efe33d5d76f3c356996f643c40c1cf0736cf7bc0 Mon Sep 17 00:00:00 2001
From: Moh'd Khier Abualruz <mkh.ruzz@live.com>
Date: Tue, 21 Feb 2023 22:19:52 +0100
Subject: [PATCH] Ensure ToC is always accessible via landmark region (<nav>)

- We had to add nav element to wrap TOC in all places
- Make sure proper nav is only displayed so landmark do not get confused
- Remove Eslint block on rules compat/compat for no support IE 11, this was blocking commit
- Run lint:fix:js and lint:fix:styles
- Fix maxSize in bundlesize.config.json

Bug: T324505
Change-Id: I36d26240b87de33318ff1edf97a410872a438103
---
 bundlesize.config.json                        |  2 +-
 includes/templates/PageTitlebar.mustache      | 15 +++++-----
 includes/templates/StickyHeader.mustache      | 30 +++++++++++--------
 .../TableOfContentsContainer.mustache         |  2 +-
 .../components/TableOfContents.less           | 26 ++++++++++++++++
 .../skins.vector.styles/layouts/gradeC.less   |  4 +--
 .../__snapshots__/stickyHeader.test.js.snap   | 10 ++++---
 tests/jest/tableOfContents.test.js            |  6 ++--
 8 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/bundlesize.config.json b/bundlesize.config.json
index 077d0c671..c5209f589 100644
--- a/bundlesize.config.json
+++ b/bundlesize.config.json
@@ -5,7 +5,7 @@
 	},
 	{
 		"resourceModule": "skins.vector.styles",
-		"maxSize": "11.3 kB"
+		"maxSize": "11.5 kB"
 	},
 	{
 		"resourceModule": "skins.vector.legacy.js",
diff --git a/includes/templates/PageTitlebar.mustache b/includes/templates/PageTitlebar.mustache
index 10710fcd8..de6b31d19 100644
--- a/includes/templates/PageTitlebar.mustache
+++ b/includes/templates/PageTitlebar.mustache
@@ -17,13 +17,14 @@
 	</label>
 
 	{{! TOC is moved into this dropdown from the sidebar in stickyHeader.js. }}
-	{{#data-page-titlebar-toc-dropdown}}{{>Dropdown/Open}}{{/data-page-titlebar-toc-dropdown}}
-		{{#data-page-titlebar-toc-pinnable-container}}
-		{{>PinnableContainer/Unpinned/Open}}
-		{{>PinnableContainer/Close}}
-		{{/data-page-titlebar-toc-pinnable-container}}
-	{{#data-page-titlebar-toc-dropdown}}{{>Dropdown/Close}}{{/data-page-titlebar-toc-dropdown}}
-
+	<nav role="navigation" aria-label="{{ msg-vector-toc-label }}" class="vector-toc-landmark">
+		{{#data-page-titlebar-toc-dropdown}}{{>Dropdown/Open}}{{/data-page-titlebar-toc-dropdown}}
+			{{#data-page-titlebar-toc-pinnable-container}}
+			{{>PinnableContainer/Unpinned/Open}}
+			{{>PinnableContainer/Close}}
+			{{/data-page-titlebar-toc-pinnable-container}}
+		{{#data-page-titlebar-toc-dropdown}}{{>Dropdown/Close}}{{/data-page-titlebar-toc-dropdown}}
+	</nav>
 	{{{html-title-heading}}}
 
 	{{#is-language-in-content-top}}
diff --git a/includes/templates/StickyHeader.mustache b/includes/templates/StickyHeader.mustache
index 07aee48dc..160f13290 100644
--- a/includes/templates/StickyHeader.mustache
+++ b/includes/templates/StickyHeader.mustache
@@ -6,11 +6,13 @@
 
 	See https://dequeuniversity.com/rules/axe/4.1/aria-hidden-focus and https://phabricator.wikimedia.org/T290201
 	for more context.
+
+	The exception to this is the TOC in the sticky header, which is not duplicated and is moved into the
+	sticky header in the DOM. We apply `aria-hidden` to all the contents of the sticky header except for the TOC.
 }}
-<header id="vector-sticky-header" aria-hidden="true"
-	class="vector-sticky-header">
+<header id="vector-sticky-header" class="vector-sticky-header">
 	<div class="vector-sticky-header-start">
-		<div class="vector-sticky-header-icon-start mw-ui-icon-flush-left mw-ui-icon-flush-right">
+		<div class="vector-sticky-header-icon-start mw-ui-icon-flush-left mw-ui-icon-flush-right" aria-hidden="true">
 			{{#data-button-start}}
 			{{>Button}}
 			{{/data-button-start}}
@@ -20,18 +22,20 @@
 		{{/data-search}}
 		<div class="vector-sticky-header-context-bar">
 			{{! TOC is moved into this dropdown from the sidebar in stickyHeader.js. }}
-			{{#data-sticky-header-toc-dropdown}}
-			{{>Dropdown/Open}}
-				{{#data-sticky-header-toc-pinnable-container}}
-				{{>PinnableContainer/Unpinned/Open}}
-				{{>PinnableContainer/Close}}
-				{{/data-sticky-header-toc-pinnable-container}}
-			{{>Dropdown/Close}}
-			{{/data-sticky-header-toc-dropdown}}
-			<div class="vector-sticky-header-context-bar-primary" {{{html-user-language-attributes}}}>{{{html-title}}}</div>
+			<nav role="navigation" aria-label="{{ msg-vector-toc-label }}" class="vector-toc-landmark">
+				{{#data-sticky-header-toc-dropdown}}
+					{{>Dropdown/Open}}
+						{{#data-sticky-header-toc-pinnable-container}}
+						{{>PinnableContainer/Unpinned/Open}}
+						{{>PinnableContainer/Close}}
+						{{/data-sticky-header-toc-pinnable-container}}
+					{{>Dropdown/Close}}
+				{{/data-sticky-header-toc-dropdown}}
+			</nav>
+			<div class="vector-sticky-header-context-bar-primary" aria-hidden="true" {{{html-user-language-attributes}}}>{{{html-title}}}</div>
 		</div>
 	</div>
-	<div class="vector-sticky-header-end{{^is-page-tools-enabled}} mw-ui-icon-flush-right{{/is-page-tools-enabled}}">
+	<div class="vector-sticky-header-end{{^is-page-tools-enabled}} mw-ui-icon-flush-right{{/is-page-tools-enabled}}" aria-hidden="true">
 		<div class="vector-sticky-header-icons">
 			{{#data-buttons}}
 			{{>Button}}
diff --git a/includes/templates/TableOfContentsContainer.mustache b/includes/templates/TableOfContentsContainer.mustache
index 4726fe677..077105a42 100644
--- a/includes/templates/TableOfContentsContainer.mustache
+++ b/includes/templates/TableOfContentsContainer.mustache
@@ -1,4 +1,4 @@
-<nav id="mw-panel-toc" role="navigation" aria-label="{{ msg-vector-toc-label }}" data-event-name="ui.sidebar-toc" class="mw-table-of-contents-container">
+<nav id="mw-panel-toc" role="navigation" aria-label="{{ msg-vector-toc-label }}" data-event-name="ui.sidebar-toc" class="mw-table-of-contents-container vector-toc-landmark">
 	{{! T313060 Additional container div needed to prevent the sticky element from being siblings with the footer }}
 	{{#data-toc-pinnable-container}}
 	{{>PinnableContainer/Pinned/Open}}
diff --git a/resources/skins.vector.styles/components/TableOfContents.less b/resources/skins.vector.styles/components/TableOfContents.less
index edb7fdb39..c2833cf31 100644
--- a/resources/skins.vector.styles/components/TableOfContents.less
+++ b/resources/skins.vector.styles/components/TableOfContents.less
@@ -142,3 +142,29 @@
 .client-js body.rtl .vector-toc .vector-toc-toggle {
 	transform: rotate( 90deg );
 }
+
+// Ensure there is only 1 table of contents landmark at anytime
+.vector-toc-landmark {
+	display: none;
+}
+
+.client-js {
+	@media ( min-width: @min-width-desktop ) {
+		.vector-toc-unpinned.vector-sticky-header-visible .vector-sticky-header .vector-toc-landmark,
+		.vector-toc-unpinned:not( .vector-sticky-header-visible ) .vector-page-titlebar .vector-toc-landmark,
+		.vector-toc-pinned .mw-table-of-contents-container.vector-toc-landmark {
+			display: block;
+		}
+	}
+
+	@media ( max-width: @max-width-tablet ) {
+		// On lower resolutions the TOC will always be in the page titlebar regardless of the TOC pinned classes
+		.vector-page-titlebar .vector-toc-landmark {
+			display: block;
+		}
+
+		.mw-table-of-contents-container.vector-toc-landmark {
+			display: none;
+		}
+	}
+}
diff --git a/resources/skins.vector.styles/layouts/gradeC.less b/resources/skins.vector.styles/layouts/gradeC.less
index 8e6e5b223..dbc53465e 100644
--- a/resources/skins.vector.styles/layouts/gradeC.less
+++ b/resources/skins.vector.styles/layouts/gradeC.less
@@ -6,12 +6,12 @@
 // On IE11 this leaves a clean single column grid.
 // To be revisited when table of contents is collapsible in which situation we should be able to
 // force it as sticky.
-.mw-table-of-contents-container {
+.vector-pinned-container {
 	display: none;
 }
 
 @supports ( display: grid ) {
-	.mw-table-of-contents-container {
+	.vector-pinned-container {
 		display: block;
 	}
 }
diff --git a/tests/jest/__snapshots__/stickyHeader.test.js.snap b/tests/jest/__snapshots__/stickyHeader.test.js.snap
index 0caf79dec..670a17f6a 100644
--- a/tests/jest/__snapshots__/stickyHeader.test.js.snap
+++ b/tests/jest/__snapshots__/stickyHeader.test.js.snap
@@ -1,9 +1,9 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`Sticky header renders 1`] = `
-"<header id=\\"vector-sticky-header\\" aria-hidden=\\"true\\" class=\\"vector-sticky-header\\">
+"<header id=\\"vector-sticky-header\\" class=\\"vector-sticky-header\\">
 	<div class=\\"vector-sticky-header-start\\">
-		<div class=\\"vector-sticky-header-icon-start mw-ui-icon-flush-left mw-ui-icon-flush-right\\">
+		<div class=\\"vector-sticky-header-icon-start mw-ui-icon-flush-left mw-ui-icon-flush-right\\" aria-hidden=\\"true\\">
 
 <button tabindex=\\"-1\\" data-event-name=\\"ui.vector-sticky-search-form.icon\\" class=\\"mw-ui-button mw-ui-quiet mw-ui-icon mw-ui-icon-element mw-ui-icon-wikimedia-search vector-sticky-header-search-toggle\\">
 	<span>search</span>
@@ -11,6 +11,7 @@ exports[`Sticky header renders 1`] = `
 </button>
 		</div>
 <div> </div>		<div class=\\"vector-sticky-header-context-bar\\">
+			<nav role=\\"navigation\\" aria-label=\\"\\" class=\\"vector-toc-landmark\\">
 <div id=\\"vector-sticky-header-toc\\" class=\\"vector-menu vector-dropdown vector-menu-dropdown mw-portlet mw-portlet-sticky-header-toc vector-sticky-header-toc\\">
 	<input type=\\"checkbox\\" id=\\"vector-sticky-header-toc-checkbox\\" role=\\"button\\" aria-haspopup=\\"true\\" data-event-name=\\"ui.dropdown-vector-sticky-header-toc\\" class=\\"vector-menu-checkbox\\" aria-label=\\"\\" tabindex=\\"-1\\">
 	<label id=\\"vector-sticky-header-toc-label\\" for=\\"vector-sticky-header-toc-checkbox\\" class=\\"vector-menu-heading mw-ui-button mw-ui-quiet mw-ui-icon mw-ui-icon-element mw-ui-icon-wikimedia-listBullet\\" aria-hidden=\\"true\\" tabindex=\\"-1\\">
@@ -19,10 +20,11 @@ exports[`Sticky header renders 1`] = `
 	<div class=\\"vector-menu-content vector-dropdown-content\\">
 
 	</div>
-</div>			<div class=\\"vector-sticky-header-context-bar-primary\\"></div>
+</div>			</nav>
+			<div class=\\"vector-sticky-header-context-bar-primary\\" aria-hidden=\\"true\\"></div>
 		</div>
 	</div>
-	<div class=\\"vector-sticky-header-end mw-ui-icon-flush-right\\">
+	<div class=\\"vector-sticky-header-end mw-ui-icon-flush-right\\" aria-hidden=\\"true\\">
 		<div class=\\"vector-sticky-header-icons\\">
 <a href=\\"#\\" id=\\"ca-talk-sticky-header\\" data-event-name=\\"talk-sticky-header\\" class=\\"mw-ui-button mw-ui-quiet mw-ui-icon mw-ui-icon-element mw-ui-icon-wikimedia-speechBubbles sticky-header-icon\\">
 	<span></span>
diff --git a/tests/jest/tableOfContents.test.js b/tests/jest/tableOfContents.test.js
index fbd9c611e..87f534939 100644
--- a/tests/jest/tableOfContents.test.js
+++ b/tests/jest/tableOfContents.test.js
@@ -85,9 +85,9 @@ function render( templateProps = {} ) {
 	}, templateProps );
 
 	return mustache.render( tableOfContentsTemplate, templateData, {
-		'PinnableElement/Open': pinnableElementOpenTemplate, // eslint-disable-line camelcase
-		'PinnableElement/Close': pinnableElementCloseTemplate, // eslint-disable-line camelcase
-		PinnableHeader: pinnableHeaderTemplate, // eslint-disable-line camelcase
+		'PinnableElement/Open': pinnableElementOpenTemplate,
+		'PinnableElement/Close': pinnableElementCloseTemplate,
+		PinnableHeader: pinnableHeaderTemplate,
 		TableOfContents__list: tableOfContentsContentsTemplate, // eslint-disable-line camelcase
 		TableOfContents__line: tableOfContentsLineTemplate // eslint-disable-line camelcase
 	} );