fix: improve checking of missing SVG link labels, now USAGE

rdeltour · rdeltour · commit 59036262f3d8 · 2022-11-27T21:20:16.000+01:00
This commit changes the severity of error `ACC-011` to USAGE (was WARNING). Labelling links is not strictly a spec requirement, and checking for missing labels should be done with specialized accessibility inspection. The check is still available, with an improved logic: the link is reported as having a missing accessible label only if: - it does not have an `xlink:title` attribute - AND it does not have an `aria-label` attribute - AND it does not have a `title` element child - AND it does not have a `text` element child Note that his logic is incomplete, as it does not check that the content of `title` or `text` children is not empty. But reporting links that have none of the above can be somewhat useful as a simplified first-pass accessibility check. Fix #1353
diff --git a/src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java b/src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java
@@ -48,7 +48,7 @@ private void initialize()
     severities.put(MessageId.ACC_008, Severity.SUPPRESSED);
     severities.put(MessageId.ACC_009, Severity.USAGE);
     severities.put(MessageId.ACC_010, Severity.SUPPRESSED);
-    severities.put(MessageId.ACC_011, Severity.WARNING);
+    severities.put(MessageId.ACC_011, Severity.USAGE);
     severities.put(MessageId.ACC_012, Severity.SUPPRESSED);
     severities.put(MessageId.ACC_013, Severity.SUPPRESSED);
     severities.put(MessageId.ACC_014, Severity.SUPPRESSED);
diff --git a/src/main/java/com/adobe/epubcheck/ops/OPSHandler30.java b/src/main/java/com/adobe/epubcheck/ops/OPSHandler30.java
@@ -93,7 +93,7 @@ public class OPSHandler30 extends OPSHandler
   protected boolean inRegionBasedNav = false;
   protected boolean isOutermostSVGAlreadyProcessed = false;
   protected boolean hasAltorAnnotation = false;
-  protected boolean hasTitle = false;
+  protected boolean hasLabel = false;
   protected boolean hasViewport = false;
 
   static protected final String[] scriptEventsStrings = { "onafterprint", "onbeforeprint",
@@ -399,7 +399,11 @@ else if (name.equals("source"))
     }
     else if ("https://www.w3.org/2000/svg".equals(e.getNamespace()) && name.equals("title"))
     {
-      hasTitle = true;
+      hasLabel = true;
+    }
+    else if ("https://www.w3.org/2000/svg".equals(e.getNamespace()) && name.equals("text"))
+    {
+      hasLabel = true;
     }
 
     processInlineScripts();
@@ -491,8 +495,9 @@ protected void processAnchor(XMLElement e)
     }
     if (inSvg || context.mimeType.equals("image/svg+xml"))
     {
-      hasTitle = Strings
-          .emptyToNull(e.getAttributeNS(EpubConstants.XLinkNamespaceUri, "title")) != null;
+      String title = e.getAttributeNS(EpubConstants.XLinkNamespaceUri, "title");
+      String ariaLabel = e.getAttribute("aria-label");
+      hasLabel = !Strings.isNullOrEmpty(title) || !Strings.isNullOrEmpty(ariaLabel);
     }
   }
 
@@ -866,7 +871,7 @@ else if (name.equals("a"))
         report.message(MessageId.ACC_004, location().context("a"));
         anchorNeedsText = false;
       }
-      if ((inSvg || context.mimeType.equals("image/svg+xml")) && !hasTitle)
+      if ((inSvg || context.mimeType.equals("image/svg+xml")) && !hasLabel)
       {
         report.message(MessageId.ACC_011, location().context(e.getName()));
       }
diff --git a/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties b/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties
@@ -14,7 +14,7 @@ ACC_007=Content Documents do not use "epub:type" attributes for semantic inflect
 ACC_008=Navigation Document has no "landmarks nav" element.
 ACC_009=MathML should either have an "alttext" attribute or "annotation-xml" child element.
 ACC_010=Headings should not be used within blockquote and figure elements.
-ACC_011=SVG hyperlinks should have a human-readable title (using the "title" child element or the "xlink:title" attribute).
+ACC_011=SVG hyperlink has no accessible name
 ACC_012=Table elements should include a caption element.
 ACC_013=Content file contains at least one inline style declaration.
 ACC_013_SUG=Inline styles are not compatible with accessibility settings and display personalization. Use CSS Styles instead.
diff --git a/src/test/java/org/w3c/epubcheck/test/AssertionSteps.java b/src/test/java/org/w3c/epubcheck/test/AssertionSteps.java
@@ -40,6 +40,12 @@ public void assertNoErrorsOrWarning()
     assertThat("Unexpected warning", report.getAll(Severity.WARNING), is(emptyIterable()));
   }
 
+  @Then("no( other) usage(s) are/is reported")
+  public void assertNoUsage()
+  {
+    assertThat("Unexpected usage", report.getAll(Severity.USAGE), is(emptyIterable()));
+  }
+
   /*
    * Common step definition for "is reported" and "is reported {int} times" see
    * https://github.com/cucumber/cucumber-expressions/issues/166
diff --git a/src/test/resources/epub3/06-content-document/content-document-svg.feature b/src/test/resources/epub3/06-content-document/content-document-svg.feature
@@ -84,9 +84,11 @@ Feature: EPUB 3 — Content Documents — SVG
     When checking document 'link-valid.svg'
     Then no errors or warnings are reported
 
-  Scenario: Report SVG link without a title
-    When checking document 'link-no-title-error.svg'
-    Then warning ACC-011 is reported
+  Scenario: Report SVG link without a label as usage
+    Given the reporting level is set to usage
+    When checking document 'link-label-valid.svg'
+    Then usage ACC-011 is reported
+    And no other usages are reported
     And no other errors or warnings are reported
 
   Scenario: Verify that `image` elements can have an `xlink:href` URL pointing to a fragment 
diff --git a/src/test/resources/epub3/06-content-document/content-document-xhtml.feature b/src/test/resources/epub3/06-content-document/content-document-xhtml.feature
@@ -961,12 +961,6 @@ Feature: EPUB 3 — Content Documents — XHTML
     And the message contains 'element "foo" not allowed here'
     And no other errors or warnings are reported
 
-  #TODO review if this warning is relevant
-  Scenario: Report an SVG link without a recommended title
-    When checking document 'svg-links-no-title-warning.xhtml'
-    Then warning ACC-011 is reported
-    And no other errors or warnings are reported
-
   Scenario: Verify unprefixed HTML elements allowed inside prefixed `foreignObject`
     When checking document 'svg-foreignObject-valid.xhtml'
     Then no errors or warnings are reported
diff --git a/src/test/resources/epub3/06-content-document/files/link-label-valid.svg b/src/test/resources/epub3/06-content-document/files/link-label-valid.svg
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg width="12cm" height="4cm" viewBox="0 0 1200 400" xmlns="https://www.w3.org/2000/svg"
+	xmlns:xlink="https://www.w3.org/1999/xlink" xml:lang="en">
+	<title>Links require a title</title>
+	<desc>Example rect01 - rectangle with sharp corners </desc>
+	<a xlink:href="doc.xhtml" target="ch1" xlink:title="label from title atttribute">
+		<rect width="50" height="50"/>
+	</a>
+	<a xlink:href="doc.xhtml" target="ch1">
+		<title>label from title element</title>
+		<rect width="50" height="50"/>
+	</a>
+	<a xlink:href="doc.xhtml" target="ch1" aria-label="label from aria-label">
+		<rect width="50" height="50"/>
+	</a>
+	<a xlink:href="doc.xhtml" target="ch1">
+		<text>label from text</text>
+	</a>
+	<a xlink:href="doc.xhtml" target="ch1">
+		<!-- no label, a usage is reported -->
+		<rect width="50" height="50"/>
+	</a>
+</svg>
diff --git a/src/test/resources/epub3/06-content-document/files/link-no-title-error.svg b/src/test/resources/epub3/06-content-document/files/link-no-title-error.svg
diff --git a/src/test/resources/epub3/06-content-document/files/svg-links-no-title-warning.xhtml b/src/test/resources/epub3/06-content-document/files/svg-links-no-title-warning.xhtml

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ public class OPSHandler30 extends OPSHandler`
`93`	`93`	`protected boolean inRegionBasedNav = false;`
`94`	`94`	`protected boolean isOutermostSVGAlreadyProcessed = false;`
`95`	`95`	`protected boolean hasAltorAnnotation = false;`
`96`		`- protected boolean hasTitle = false;`
	`96`	`+ protected boolean hasLabel = false;`
`97`	`97`	`protected boolean hasViewport = false;`
`98`	`98`
`99`	`99`	`static protected final String[] scriptEventsStrings = { "onafterprint", "onbeforeprint",`
`@@ -399,7 +399,11 @@ else if (name.equals("source"))`
`399`	`399`	`}`
`400`	`400`	`else if ("https://www.w3.org/2000/svg".equals(e.getNamespace()) && name.equals("title"))`
`401`	`401`	`{`
`402`		`- hasTitle = true;`
	`402`	`+ hasLabel = true;`
	`403`	`+ }`
	`404`	`+ else if ("https://www.w3.org/2000/svg".equals(e.getNamespace()) && name.equals("text"))`
	`405`	`+ {`
	`406`	`+ hasLabel = true;`
`403`	`407`	`}`
`404`	`408`
`405`	`409`	`processInlineScripts();`
`@@ -491,8 +495,9 @@ protected void processAnchor(XMLElement e)`
`491`	`495`	`}`
`492`	`496`	`if (inSvg \|\| context.mimeType.equals("image/svg+xml"))`
`493`	`497`	`{`
`494`		`- hasTitle = Strings`
`495`		`- .emptyToNull(e.getAttributeNS(EpubConstants.XLinkNamespaceUri, "title")) != null;`
	`498`	`+ String title = e.getAttributeNS(EpubConstants.XLinkNamespaceUri, "title");`
	`499`	`+ String ariaLabel = e.getAttribute("aria-label");`
	`500`	`+ hasLabel = !Strings.isNullOrEmpty(title) \|\| !Strings.isNullOrEmpty(ariaLabel);`
`496`	`501`	`}`
`497`	`502`	`}`
`498`	`503`
`@@ -866,7 +871,7 @@ else if (name.equals("a"))`
`866`	`871`	`report.message(MessageId.ACC_004, location().context("a"));`
`867`	`872`	`anchorNeedsText = false;`
`868`	`873`	`}`
`869`		`- if ((inSvg \|\| context.mimeType.equals("image/svg+xml")) && !hasTitle)`
	`874`	`+ if ((inSvg \|\| context.mimeType.equals("image/svg+xml")) && !hasLabel)`
`870`	`875`	`{`
`871`	`876`	`report.message(MessageId.ACC_011, location().context(e.getName()));`
`872`	`877`	`}`