From 687c6a50d866dc32922885ef9cdbe16faf4ca656 Mon Sep 17 00:00:00 2001
From: bwang <bwang@wikimedia.org>
Date: Tue, 18 May 2021 13:27:33 -0500
Subject: [PATCH] Improve heading structure and heading semantics by removing
 redundant headings and labels for modern Vector

- Apply aria-hidden to h3 in Menu template, ensuring the nav landmarks are still properly labeled, but no longer reading duplicate labels to screenreaders
- Remove "Navigation menu" h2 element, which is not needed as the nav landmarks are already labeled
- Remove searchbox h3 element, which is redundant with the search landmark
- Scope all changes to modern vector

Bug: T265993
Change-Id: I4e5da7a0699160fa57234bd126b75243e0157778
---
 includes/SkinVector.php                                   | 2 ++
 includes/templates/Menu.mustache                          | 5 +++--
 includes/templates/Navigation.mustache                    | 1 -
 includes/templates/SearchBox.mustache                     | 8 +++++---
 includes/templates/skin-legacy.mustache                   | 3 +--
 includes/templates/skin.mustache                          | 1 -
 resources/common/components/SearchBox.less                | 5 -----
 .../skins.vector.styles.legacy/components/SearchBox.less  | 6 ++++++
 resources/skins.vector.styles.legacy/layouts/screen.less  | 6 ------
 resources/skins.vector.styles.legacy/skin-legacy.less     | 1 +
 resources/skins.vector.styles/layouts/screen.less         | 6 ------
 11 files changed, 18 insertions(+), 26 deletions(-)
 create mode 100644 resources/skins.vector.styles.legacy/components/SearchBox.less

diff --git a/includes/SkinVector.php b/includes/SkinVector.php
index 8f3c5e764..e3958762a 100644
--- a/includes/SkinVector.php
+++ b/includes/SkinVector.php
@@ -345,6 +345,8 @@ class SkinVector extends SkinMustache {
 		// Conditionally used values must use null to indicate absence (not false or '').
 
 		$commonSkinData = array_merge( $parentData, [
+			'is-legacy' => $this->isLegacy(),
+
 			'is-article' => (bool)$out->isArticle(),
 
 			'is-anon' => $this->getUser()->isAnon(),
diff --git a/includes/templates/Menu.mustache b/includes/templates/Menu.mustache
index c47847754..81c30a13a 100644
--- a/includes/templates/Menu.mustache
+++ b/includes/templates/Menu.mustache
@@ -12,8 +12,9 @@
 		data-event-name="ui.dropdown-{{id}}"
 		class="{{#checkbox-class}}{{.}}{{/checkbox-class}} vector-menu-checkbox" aria-labelledby="{{id}}-label" />
 	{{/is-dropdown}}
-	<h3 id="{{id}}-label" {{#heading-class}}class="{{.}}"{{/heading-class}}>{{!
-		}}{{{html-vector-heading-icon}}} <span>{{label}}</span>
+	<h3 id="{{id}}-label" {{#heading-class}}class="{{.}}"{{/heading-class}} {{^is-legacy}}aria-hidden="true"{{/is-legacy}}>
+		{{{html-vector-heading-icon}}}
+		<span>{{label}}</span>
 		{{#is-dropdown}}
 			<span class="vector-menu-checkbox-expanded">{{msg-vector-menu-checkbox-expanded}}</span>
 			<span class="vector-menu-checkbox-collapsed">{{msg-vector-menu-checkbox-collapsed}}</span>
diff --git a/includes/templates/Navigation.mustache b/includes/templates/Navigation.mustache
index 5bbd5e99a..9eac40640 100644
--- a/includes/templates/Navigation.mustache
+++ b/includes/templates/Navigation.mustache
@@ -1,5 +1,4 @@
 <div id="mw-navigation">
-	<h2>{{msg-navigation-heading}}</h2>
 	{{#data-portlets-sidebar}}{{>Sidebar}}{{/data-portlets-sidebar}}
 	<div id="mw-head">
 		<div class="mw-article-toolbar-container">
diff --git a/includes/templates/SearchBox.mustache b/includes/templates/SearchBox.mustache
index 70e6eba84..478791b0b 100644
--- a/includes/templates/SearchBox.mustache
+++ b/includes/templates/SearchBox.mustache
@@ -3,9 +3,11 @@
 }}
 <div id="p-search" role="search" class="{{class}}">
 	<div>
-		<h3 {{{html-user-language-attributes}}}>
-			<label for="searchInput">{{msg-search}}</label>
-		</h3>
+		{{#is-legacy}}
+			<h3 {{{html-user-language-attributes}}}>
+				<label for="searchInput">{{msg-search}}</label>
+			</h3>
+		{{/is-legacy}}
 		<form action="{{form-action}}" id="searchform">
 			<div id="simpleSearch"{{#input-location}} data-search-loc="{{.}}"{{/input-location}}>
 				{{{html-input}}}
diff --git a/includes/templates/skin-legacy.mustache b/includes/templates/skin-legacy.mustache
index 274e8ac0d..3ac649275 100644
--- a/includes/templates/skin-legacy.mustache
+++ b/includes/templates/skin-legacy.mustache
@@ -14,8 +14,7 @@
 	string html-body-content
 	string html-categories
 	string html-after-content
-	string msg-navigation-heading heading for entire navigation that is
-		usually hidden to screen readers
+	string msg-navigation-heading
 	MenuDefinition data-portlets.data-personal
 	MenuDefinition data-portlets.data-namespaces
 	MenuDefinition data-portlets.data-variants
diff --git a/includes/templates/skin.mustache b/includes/templates/skin.mustache
index 7f44c1316..44ee02d05 100644
--- a/includes/templates/skin.mustache
+++ b/includes/templates/skin.mustache
@@ -15,7 +15,6 @@
 	string html-body-content
 	string html-categories
 	string html-after-content
-	string msg-navigation-heading
 	LogoOptions data-logos
 	object data-portlets
 	MenuDefinition data-portlets.data-personal
diff --git a/resources/common/components/SearchBox.less b/resources/common/components/SearchBox.less
index 44d22664c..4b10514f6 100644
--- a/resources/common/components/SearchBox.less
+++ b/resources/common/components/SearchBox.less
@@ -2,11 +2,6 @@
 @import 'mediawiki.ui/variables.less';
 @import '../../common/variables.less';
 
-// Search portlet.
-#p-search h3 {
-	.mixin-screen-reader-text();
-}
-
 // Defined as `div`.
 // Provide extra element for gadgets due to `form` already carrying an `id`.
 #simpleSearch {
diff --git a/resources/skins.vector.styles.legacy/components/SearchBox.less b/resources/skins.vector.styles.legacy/components/SearchBox.less
new file mode 100644
index 000000000..b7e81597f
--- /dev/null
+++ b/resources/skins.vector.styles.legacy/components/SearchBox.less
@@ -0,0 +1,6 @@
+@import 'mediawiki.mixins.less';
+
+// Search portlet.
+#p-search h3 {
+	.mixin-screen-reader-text();
+}
diff --git a/resources/skins.vector.styles.legacy/layouts/screen.less b/resources/skins.vector.styles.legacy/layouts/screen.less
index b81ccd47b..1bdfb02ab 100644
--- a/resources/skins.vector.styles.legacy/layouts/screen.less
+++ b/resources/skins.vector.styles.legacy/layouts/screen.less
@@ -81,12 +81,6 @@ body {
 	height: 5em;
 }
 
-/* Hide, but keep accessible for screen-readers */
-#mw-navigation h2 {
-	position: absolute;
-	top: -9999px;
-}
-
 #mw-head {
 	position: absolute;
 	top: 0;
diff --git a/resources/skins.vector.styles.legacy/skin-legacy.less b/resources/skins.vector.styles.legacy/skin-legacy.less
index 50ba84842..4e7ae76bc 100644
--- a/resources/skins.vector.styles.legacy/skin-legacy.less
+++ b/resources/skins.vector.styles.legacy/skin-legacy.less
@@ -9,6 +9,7 @@
 
 	// Legacy specific components
 	@import './components/MenuDropdown.less';
+	@import './components/SearchBox.less';
 	@import './components/Sidebar.less';
 	@import './components/UserLinks.less';
 }
diff --git a/resources/skins.vector.styles/layouts/screen.less b/resources/skins.vector.styles/layouts/screen.less
index 32a6472cf..a5f92c94c 100644
--- a/resources/skins.vector.styles/layouts/screen.less
+++ b/resources/skins.vector.styles/layouts/screen.less
@@ -198,12 +198,6 @@ body {
 	z-index: @z-index-base;
 }
 
-/* Hide, but keep accessible for screen-readers */
-#mw-navigation h2 {
-	position: absolute;
-	top: -9999px;
-}
-
 /* Navigation Containers */
 .mw-article-toolbar-container {
 	// Clear the floats on #left-navigation and #right-navigation.