feat: Support aria-roledescription (also fix forms detection) (#317)

* Support aria-roledescription (closes #316)
* Fix form labelling requirements in line with recent developments in matatk/page-structural-semantics-scanner-tests#2

package.json

@@ -35,7 +35,7 @@
     "fs-extra": "~7.0",
     "husky": "~1.3",
     "one-svg-to-many-sized-pngs": "github:matatk/one-svg-to-many-sized-pngs#0.1.1",
-    "page-structural-semantics-scanner-tests": "git+https://github.com/matatk/page-structural-semantics-scanner-tests.git#0.2.0",
+    "page-structural-semantics-scanner-tests": "git+https://github.com/matatk/page-structural-semantics-scanner-tests.git#0.3.0",
     "puppeteer": "~1.14",
     "replace-in-file": "~3.4",
     "rollup": "~1.10",

src/code/_content.js

@@ -34,23 +34,23 @@ function messageHandler(message) {
 			// of the keyboard shortcuts (if landmarks are present)
 			checkAndUpdateOutdatedResults()
 			checkFocusElement(() =>
-				landmarksFinder.getLandmarkElementRoleLabel(message.index))
+				landmarksFinder.getLandmarkElementInfo(message.index))
 			break
 		case 'next-landmark':
 			// Triggered by keyboard shortcut
 			checkAndUpdateOutdatedResults()
-			checkFocusElement(landmarksFinder.getNextLandmarkElementRoleLabel)
+			checkFocusElement(landmarksFinder.getNextLandmarkElementInfo)
 			break
 		case 'prev-landmark':
 			// Triggered by keyboard shortcut
 			checkAndUpdateOutdatedResults()
 			checkFocusElement(
-				landmarksFinder.getPreviousLandmarkElementRoleLabel)
+				landmarksFinder.getPreviousLandmarkElementInfo)
 			break
 		case 'main-landmark': {
 			// Triggered by keyboard shortcut
 			checkAndUpdateOutdatedResults()
-			const mainElementInfo = landmarksFinder.getMainElementRoleLabel()
+			const mainElementInfo = landmarksFinder.getMainElementInfo()
 			if (mainElementInfo) {
 				elementFocuser.focusElement(mainElementInfo)
 			} else {
@@ -65,7 +65,7 @@ function messageHandler(message) {
 				if (elementFocuser.isManagingBorders()) {
 					elementFocuser.manageBorders(false)
 					borderDrawer.addBorderToElements(
-						landmarksFinder.allElementsRolesLabels())
+						landmarksFinder.allElementsInfos())
 				} else {
 					borderDrawer.removeAllBorders()
 					elementFocuser.manageBorders(true)
@@ -131,7 +131,7 @@ function checkFocusElement(callbackReturningElementInfo) {
 function sendLandmarks() {
 	browser.runtime.sendMessage({
 		name: 'landmarks',
-		data: landmarksFinder.allDepthsRolesLabelsSelectors()
+		data: landmarksFinder.allInfos()
 	})
 }
 
@@ -146,7 +146,7 @@ function findLandmarksAndUpdateExtension() {
 	borderDrawer.refreshBorders()
 	if (!elementFocuser.isManagingBorders()) {
 		borderDrawer.addBorderToElements(
-			landmarksFinder.allElementsRolesLabels())
+			landmarksFinder.allElementsInfos())
 	}
 }
 

src/code/_gui.js

@@ -13,11 +13,6 @@ let port = null
 
 // Handle incoming landmarks message response
 //
-// If there are landmarks, then the response will be a list of objects that
-// represent the landmarks.
-//
-//     [ { label: X, role: Y, depth: Z, selector: @ }, { . . . }, . . . ]
-//
 // If we got some landmarks from the page, make the tree of them. If there was
 // an error, let the user know.
 function handleLandmarksMessage(data) {

src/code/borderDrawer.js

@@ -66,8 +66,6 @@ export default function BorderDrawer(win, doc, contrastChecker) {
 	// Add the landmark border and label for an element. Takes an element info
 	// object, as returned by the various LandmarksFinder functions.
 	//
-	// { element: <HTMLElement>, role: <string>, label: <string> }
-	//
 	// Note: we assume that if an element already exists and we try to add it
 	//       again (as may happen if the page changes whilst we're displaying
 	//       all elements, and try to add any new ones) that the existing

src/code/elementFocuser.js

@@ -36,8 +36,6 @@ export default function ElementFocuser(doc, borderDrawer) {
 	// Set focus on the selected landmark element. It takes an element info
 	// object, as returned by the various LandmarksFinder functions.
 	//
-	// { element: <HTMLElement>, role: <string>, label: <string> }
-	//
 	// Note: this should only be called if landmarks were found. The check
 	//       for this is done in the main content script, as it involves UI
 	//       activity, and couples finding and focusing.

src/code/landmarkName.js

@@ -1,11 +1,13 @@
-// If the landmark has a label, the name is: 'label (role)'
-// otherwise the name is just 'role'
 export default function landmarkName(landmark) {
+	const roleName = landmark.roleDescription
+		? landmark.roleDescription
+		: processRole(landmark.role)
+
 	if (landmark.label) {
-		return landmark.label + ' (' + processRole(landmark.role) + ')'
+		return landmark.label + ' (' + roleName + ')'
 	}
 
-	return processRole(landmark.role)
+	return roleName
 }
 
 // Fetch the user-friendly name for a role

src/code/landmarksFinder.js

@@ -9,10 +9,10 @@ export default function LandmarksFinder(win, doc) {
 		'banner',
 		'complementary',
 		'contentinfo',
-		'form',           // spec says should label; Landmarks ignores if not
+		'form',           // spec says should label
 		'main',
 		'navigation',
-		'region',         // spec says must label; Landmarks ignores if not
+		'region',         // spec says must label
 		'search',
 
 		// Digital Publishing ARIA module
@@ -77,11 +77,12 @@ export default function LandmarksFinder(win, doc) {
 
 	let landmarks = []
 	// Each member of this array is an object of the form:
-	//   depth: (int)            -- indicates nesting of landmarks
-	//   role: (string)          -- the ARIA role
-	//   label: (string or null) -- author-supplied label
-	//   element: (HTML*Element) -- in-memory element
-	//   selector: (string)      -- CSS selector path for this element
+	//   depth: (int)                      -- indicates nesting of landmarks
+	//   role: (string)                    -- the ARIA role
+	//   roleDescription: (string | null)  -- custom role description
+	//   label: (string | null)            -- associated label
+	//   selector: (string)                -- CSS selector path of element
+	//   element: (HTML*Element)           -- in-memory element
 
 
 	//
@@ -102,6 +103,7 @@ export default function LandmarksFinder(win, doc) {
 		return {
 			element: currentlySelectedElement,
 			role: landmarks[index].role,
+			roleDescription: landmarks[index].roleDescription,
 			label: landmarks[index].label
 			// No need to send the selector this time
 		}
@@ -118,27 +120,30 @@ export default function LandmarksFinder(win, doc) {
 
 		// Support HTML5 elements' native roles
 		let role = getRoleFromTagNameAndContainment(element)
+		let explicitRole = false
 
 		// Elements with explicitly-set rolees
 		if (element.getAttribute) {
 			const tempRole = element.getAttribute('role')
 			if (tempRole) {
 				role = tempRole
+				explicitRole = true
 			}
 		}
 
 		// The element may or may not have a label
 		const label = getARIAProvidedLabel(element)
 
 		// Add the element if it should be considered a landmark
-		if (role && isLandmark(role, label)) {
+		if (role && isLandmark(role, explicitRole, label)) {
 			if (parentLandmark && parentLandmark.contains(element)) {
 				depth = depth + 1
 			}
 
 			landmarks.push({
 				'depth': depth,
 				'role': role,
+				'roleDescription': getRoleDescription(element),
 				'label': label,
 				'element': element,
 				'selector': createSelector(element)
@@ -167,6 +172,7 @@ export default function LandmarksFinder(win, doc) {
 	function getARIAProvidedLabel(element) {
 		let label = null
 
+		// TODO general whitespace test?
 		const idRefs = element.getAttribute('aria-labelledby')
 		if (idRefs !== null && idRefs.length > 0) {
 			const innerTexts = Array.from(idRefs.split(' '), idRef => {
@@ -183,9 +189,10 @@ export default function LandmarksFinder(win, doc) {
 		return label
 	}
 
-	function isLandmark(role, label) {
-		// Region and form are landmarks only when they have labels
-		if (role === 'region' || role === 'form') {
+	function isLandmark(role, explicitRole, label) {
+		// <section> and <form> are only landmarks when labelled.
+		// <div role="form"> is always a landmark.
+		if (role === 'region' || (role === 'form' && !explicitRole)) {
 			return label !== null
 		}
 
@@ -227,6 +234,15 @@ export default function LandmarksFinder(win, doc) {
 		return role
 	}
 
+	function getRoleDescription(element) {
+		const roleDescription = element.getAttribute('aria-roledescription')
+		// TODO make this a general whitespace check?
+		if (/^\s*$/.test(roleDescription)) {
+			return null
+		}
+		return roleDescription
+	}
+
 	function isChildOfTopLevelSection(element) {
 		let ancestor = element.parentNode
 
@@ -315,42 +331,45 @@ export default function LandmarksFinder(win, doc) {
 		return landmarks.length
 	}
 
-	this.allDepthsRolesLabelsSelectors = function() {
+	this.allInfos = function() {
 		return landmarks.map(landmark => ({
 			depth: landmark.depth,
 			role: landmark.role,
+			roleDescription: landmark.roleDescription,
 			label: landmark.label,
 			selector: landmark.selector
 		}))
 	}
 
-	this.allElementsRolesLabels = function() {
+	this.allElementsInfos = function() {
 		return landmarks.map(landmark => ({
 			element: landmark.element,
+			depth: landmark.depth,
 			role: landmark.role,
-			label: landmark.label
+			roleDescription: landmark.roleDescription,
+			label: landmark.label,
+			selector: landmark.selector
 		}))
 	}
 
-	// These all return elements and their public-facing info:
-	// { element: <HTMLElement>, role: <string>, label: <string> }
+	// These all return elements and their related info
 
-	this.getNextLandmarkElementRoleLabel = function() {
+	this.getNextLandmarkElementInfo = function() {
 		return updateSelectedIndexAndReturnElementInfo(
 			(currentlySelectedIndex + 1) % landmarks.length)
 	}
 
-	this.getPreviousLandmarkElementRoleLabel = function() {
+	this.getPreviousLandmarkElementInfo = function() {
 		return updateSelectedIndexAndReturnElementInfo(
 			(currentlySelectedIndex <= 0) ?
 				landmarks.length - 1 : currentlySelectedIndex - 1)
 	}
 
-	this.getLandmarkElementRoleLabel = function(index) {
+	this.getLandmarkElementInfo = function(index) {
 		return updateSelectedIndexAndReturnElementInfo(index)
 	}
 
-	this.getMainElementRoleLabel = function() {
+	this.getMainElementInfo = function() {
 		return mainElementIndex < 0 ?
 			null : updateSelectedIndexAndReturnElementInfo(mainElementIndex)
 	}

test/test-landmarks.js

@@ -5,5 +5,5 @@ const LandmarksFinder = require('./test-code-in-harness-landmarks')
 runner(function(win, doc) {
 	const lf = new LandmarksFinder(win, doc)
 	lf.find()
-	return lf.allDepthsRolesLabelsSelectors()
+	return lf.allInfos()
 })