From b405af10a1a582d074a9a2f32a4630c08b356208 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Sat, 3 Mar 2018 18:07:24 +0100 Subject: [PATCH] fix(main-is-top-level): Rename check to landmark-is-top-level for greater reuse BREAKING CHANGE: The check main-is-top-level is no longer available --- doc/rule-descriptions.md | 4 +- lib/checks/keyboard/banner-is-top-level.js | 19 ---- lib/checks/keyboard/banner-is-top-level.json | 11 --- .../keyboard/contentinfo-is-top-level.js | 19 ---- .../keyboard/contentinfo-is-top-level.json | 11 --- ...-top-level.js => landmark-is-top-level.js} | 5 ++ .../keyboard/landmark-is-top-level.json | 11 +++ lib/checks/keyboard/main-is-top-level.json | 11 --- lib/rules/landmark-banner-is-top-level.json | 7 +- .../landmark-contentinfo-is-top-level.json | 7 +- lib/rules/landmark-has-body-context.js | 6 ++ lib/rules/landmark-main-is-top-level.json | 4 +- test/checks/keyboard/banner-is-top-level.js | 89 ------------------- .../keyboard/contentinfo-is-top-level.js | 89 ------------------- ...-top-level.js => landmark-is-top-level.js} | 46 ++++++---- .../frames/level2.html | 2 +- .../landmark-banner-is-top-level-pass.js | 2 +- .../frames/level2.html | 2 +- .../landmark-contentinfo-is-top-level-pass.js | 4 +- .../rule-matches/landmark-has-body-context.js | 44 +++++++++ 20 files changed, 110 insertions(+), 283 deletions(-) delete mode 100644 lib/checks/keyboard/banner-is-top-level.js delete mode 100644 lib/checks/keyboard/banner-is-top-level.json delete mode 100644 lib/checks/keyboard/contentinfo-is-top-level.js delete mode 100644 lib/checks/keyboard/contentinfo-is-top-level.json rename lib/checks/keyboard/{main-is-top-level.js => landmark-is-top-level.js} (81%) create mode 100644 lib/checks/keyboard/landmark-is-top-level.json delete mode 100644 lib/checks/keyboard/main-is-top-level.json create mode 100644 lib/rules/landmark-has-body-context.js delete mode 100644 test/checks/keyboard/banner-is-top-level.js delete mode 100644 test/checks/keyboard/contentinfo-is-top-level.js rename test/checks/keyboard/{main-is-top-level.js => landmark-is-top-level.js} (51%) create mode 100644 test/rule-matches/landmark-has-body-context.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index f70b77b21a..a2f5cc83be 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -34,8 +34,8 @@ | input-image-alt | Ensures <input type="image"> elements have alternate text | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | true | | label-title-only | Ensures that every form element is not solely labeled using the title or aria-describedby attributes | cat.forms, best-practice | true | | label | Ensures every form element has a label | cat.forms, wcag2a, wcag332, wcag131, section508, section508.22.n | true | -| landmark-banner-is-top-level | A banner landmark identifies site-oriented content at the beginning of each page within a website | best-practice | true | -| landmark-contentinfo-is-top-level | A contentinfo landmark is a way to identify common information at the bottom of each page within a website | best-practice | true | +| landmark-banner-is-top-level | The banner landmark should not be contained in another landmark | best-practice | true | +| landmark-contentinfo-is-top-level | The contentinfo landmark should not be contained in another landmark | best-practice | true | | landmark-main-is-top-level | The main landmark should not be contained in another landmark | best-practice | true | | landmark-no-duplicate-banner | Ensures the document has no more than one banner landmark | best-practice | true | | landmark-no-duplicate-contentinfo | Ensures the document has no more than one contentinfo landmark | best-practice | true | diff --git a/lib/checks/keyboard/banner-is-top-level.js b/lib/checks/keyboard/banner-is-top-level.js deleted file mode 100644 index 309f8ecc8d..0000000000 --- a/lib/checks/keyboard/banner-is-top-level.js +++ /dev/null @@ -1,19 +0,0 @@ -const landmarks = axe.commons.aria.getRolesByType('landmark'); -const sectioning = ['article', 'aside', 'main', 'navigation', 'section']; -const nodeIsHeader = node.tagName.toLowerCase() === 'header' && node.getAttribute('role') !== 'banner'; -var parent = axe.commons.dom.getComposedParent(node); - -while (parent){ - var role = parent.getAttribute('role'); - if (!role && (parent.tagName.toLowerCase() !== 'form')){ - role = axe.commons.aria.implicitRole(parent); - } - if (role && nodeIsHeader && sectioning.includes(role)){ - return true; - } - if (role && landmarks.includes(role)){ - return false; - } - parent = axe.commons.dom.getComposedParent(parent); -} -return true; diff --git a/lib/checks/keyboard/banner-is-top-level.json b/lib/checks/keyboard/banner-is-top-level.json deleted file mode 100644 index 23f116e7bf..0000000000 --- a/lib/checks/keyboard/banner-is-top-level.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "id": "banner-is-top-level", - "evaluate": "banner-is-top-level.js", - "metadata": { - "impact": "moderate", - "messages": { - "pass": "Banner landmark is top level or header element is not a banner", - "fail": "Banner landmark is not top level" - } - } -} diff --git a/lib/checks/keyboard/contentinfo-is-top-level.js b/lib/checks/keyboard/contentinfo-is-top-level.js deleted file mode 100644 index 3a62355f2b..0000000000 --- a/lib/checks/keyboard/contentinfo-is-top-level.js +++ /dev/null @@ -1,19 +0,0 @@ -const landmarks = axe.commons.aria.getRolesByType('landmark'); -const sectioning = ['article', 'aside', 'main', 'navigation', 'section']; -const nodeIsFooter = node.tagName.toLowerCase() === 'footer' && node.getAttribute('role') !== 'contentinfo'; -var parent = axe.commons.dom.getComposedParent(node); - -while (parent){ - var role = parent.getAttribute('role'); - if (!role && (parent.tagName.toLowerCase() !== 'form')){ - role = axe.commons.aria.implicitRole(parent); - } - if (role && nodeIsFooter && sectioning.includes(role)){ - return true; - } - if (role && landmarks.includes(role)){ - return false; - } - parent = axe.commons.dom.getComposedParent(parent); -} -return true; diff --git a/lib/checks/keyboard/contentinfo-is-top-level.json b/lib/checks/keyboard/contentinfo-is-top-level.json deleted file mode 100644 index aaea4b457c..0000000000 --- a/lib/checks/keyboard/contentinfo-is-top-level.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "id": "contentinfo-is-top-level", - "evaluate": "contentinfo-is-top-level.js", - "metadata": { - "impact": "moderate", - "messages": { - "pass": "Contentinfo landmark is top level or footer element is not contentinfo", - "fail": "Contentinfo landmark is not top level" - } - } -} diff --git a/lib/checks/keyboard/main-is-top-level.js b/lib/checks/keyboard/landmark-is-top-level.js similarity index 81% rename from lib/checks/keyboard/main-is-top-level.js rename to lib/checks/keyboard/landmark-is-top-level.js index 562c2d360f..b0504e1a68 100644 --- a/lib/checks/keyboard/main-is-top-level.js +++ b/lib/checks/keyboard/landmark-is-top-level.js @@ -1,5 +1,10 @@ var landmarks = axe.commons.aria.getRolesByType('landmark'); var parent = axe.commons.dom.getComposedParent(node); + +this.data({ + role: node.getAttribute('role') || axe.commons.aria.implicitRole(node) +}); + while (parent){ var role = parent.getAttribute('role'); if (!role && (parent.tagName.toLowerCase() !== 'form')){ diff --git a/lib/checks/keyboard/landmark-is-top-level.json b/lib/checks/keyboard/landmark-is-top-level.json new file mode 100644 index 0000000000..85d2f8bd22 --- /dev/null +++ b/lib/checks/keyboard/landmark-is-top-level.json @@ -0,0 +1,11 @@ +{ + "id": "landmark-is-top-level", + "evaluate": "landmark-is-top-level.js", + "metadata": { + "impact": "moderate", + "messages": { + "pass": "The {{=it.data.role }} landmark is at the top level.", + "fail": "The {{=it.data.role }} landmark is contained in another landmark." + } + } +} diff --git a/lib/checks/keyboard/main-is-top-level.json b/lib/checks/keyboard/main-is-top-level.json deleted file mode 100644 index 1f0038d336..0000000000 --- a/lib/checks/keyboard/main-is-top-level.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "id": "main-is-top-level", - "evaluate": "main-is-top-level.js", - "metadata": { - "impact": "moderate", - "messages": { - "pass": "The main landmark is at the top level.", - "fail": "The main landmark is contained in another landmark." - } - } -} diff --git a/lib/rules/landmark-banner-is-top-level.json b/lib/rules/landmark-banner-is-top-level.json index 9e421f49bf..04fc19165f 100644 --- a/lib/rules/landmark-banner-is-top-level.json +++ b/lib/rules/landmark-banner-is-top-level.json @@ -1,16 +1,17 @@ { "id": "landmark-banner-is-top-level", - "selector": "[role=banner], header", + "selector": "header:not([role]), [role=banner]", + "matches": "landmark-has-body-context.js", "tags": [ "best-practice" ], "metadata": { - "description": "A banner landmark identifies site-oriented content at the beginning of each page within a website", + "description": "The banner landmark should not be contained in another landmark", "help": "Banner landmark must be at top level" }, "all": [], "any": [ - "banner-is-top-level" + "landmark-is-top-level" ], "none": [] } diff --git a/lib/rules/landmark-contentinfo-is-top-level.json b/lib/rules/landmark-contentinfo-is-top-level.json index 6cd6586d9e..83ee261f81 100644 --- a/lib/rules/landmark-contentinfo-is-top-level.json +++ b/lib/rules/landmark-contentinfo-is-top-level.json @@ -1,16 +1,17 @@ { "id": "landmark-contentinfo-is-top-level", - "selector": "[role=contentinfo], footer", + "selector": "footer:not([role]), [role=contentinfo]", + "matches": "landmark-has-body-context.js", "tags": [ "best-practice" ], "metadata": { - "description": "A contentinfo landmark is a way to identify common information at the bottom of each page within a website", + "description": "The contentinfo landmark should not be contained in another landmark", "help": "Contentinfo landmark must be at top level" }, "all": [], "any": [ - "contentinfo-is-top-level" + "landmark-is-top-level" ], "none": [] } diff --git a/lib/rules/landmark-has-body-context.js b/lib/rules/landmark-has-body-context.js new file mode 100644 index 0000000000..9d7f0ffae3 --- /dev/null +++ b/lib/rules/landmark-has-body-context.js @@ -0,0 +1,6 @@ +const nativeScopeFilter = 'article, aside, main, nav, section'; + +// Filter elements that, within certain contexts, don't map their role. +// e.g. a