fix: revert FXL SVG rules to EPUB 3.0.1’s logic

Message changes:

- `HTM-048` now checks that outermost `svg` elements in Fixed-Layout
  Documents MUST have a `viewBox` attribute.
- remove `HTM-054` (`viewBox` recommended)
- remove `HTM-055` (height+width in percent when `viewBox` is abstent)
- remove old suppressed message `HTM-043`

Internal changes:

- move the SVG FXL logic out of the `ctc` package
- add an `Optional<OPFItem>` field to the validator context (retrieved
  from the `XRefChecker` in the builder, or absent).

See also w3c/epub-specs#1236

This PR (partially) reverts commit 17f5eee

src/main/java/com/adobe/epubcheck/ctc/EpubSVGCheck.java

@@ -1,7 +1,6 @@
 package com.adobe.epubcheck.ctc;
 
 import java.util.Hashtable;
-import java.util.regex.Pattern;
 import java.util.zip.ZipEntry;
 
 import org.w3c.dom.Document;
@@ -112,52 +111,27 @@ void checkSvgDoc(String svgDocEntry)
     Document doc = docParser.parseDocument(svgDocEntry);
     if (doc != null)
     {
-      checkViewBox(svgDocEntry, doc);
+//      checkViewBox(svgDocEntry, doc);
       checkImageXlinkHrefInline(svgDocEntry, doc);
     }
   }
 
-  // FIXME this RegEX is a bit too naïve for CSS syntax rules
-  // e.g. escape values and comments are theoretically allowed too
-  private static Pattern PIXEL_LENGTH_REGEX = Pattern.compile("\\d+(px)?");
-
-  void checkViewBox(String svgDocEntry, Document doc)
-  {
-    NodeList n = doc.getElementsByTagNameNS(svgNS, "svg");
-    if (n.getLength() > 0)
-    {
-      Element svgElement = (Element) n.item(0);
-      String viewport = svgElement.getAttributeNS(svgNS, "viewBox");
-      boolean viewportFound = (viewport != null && viewport.trim().length() > 0);
-      String height = svgElement.getAttributeNS(svgNS, "height");
-      boolean heightFound = (height != null && height.trim().length() > 0);
-      boolean isHeightInPixel = heightFound && PIXEL_LENGTH_REGEX.matcher(height.trim()).matches();
-      String width = svgElement.getAttributeNS(svgNS, "width");
-      boolean widthFound = (width != null && width.trim().length() > 0);
-      boolean isWidthInPixel = widthFound && PIXEL_LENGTH_REGEX.matcher(width.trim()).matches();
-      if (!viewportFound)
-      {
-        if (!heightFound || !widthFound)
-        {
-          report.message(MessageId.HTM_048,
-              EPUBLocation.create(svgDocEntry, XmlDocParser.getElementLineNumber(svgElement),
-                  XmlDocParser.getElementColumnNumber(svgElement)));
-        }
-        else
-        {
-          report.message(MessageId.HTM_054,
-              EPUBLocation.create(svgDocEntry, XmlDocParser.getElementLineNumber(svgElement),
-                  XmlDocParser.getElementColumnNumber(svgElement)));
-          if (!isHeightInPixel || !isWidthInPixel)
-          {
-            report.message(MessageId.HTM_055,
-                EPUBLocation.create(svgDocEntry, XmlDocParser.getElementLineNumber(svgElement),
-                    XmlDocParser.getElementColumnNumber(svgElement)));
-          }
-        }
-      }
-    }
-  }
+//  void checkViewBox(String svgDocEntry, Document doc)
+//  {
+//    NodeList n = doc.getElementsByTagNameNS(svgNS, "svg");
+//    if (n.getLength() > 0)
+//    {
+//      Element svgElement = (Element) n.item(0);
+//      String viewbox = svgElement.getAttributeNS(svgNS, "viewBox");
+//      boolean viewboxFound = (viewbox != null && viewbox.trim().length() > 0);
+//      if (!viewboxFound)
+//      {
+//        report.message(MessageId.HTM_048,
+//            EPUBLocation.create(svgDocEntry, XmlDocParser.getElementLineNumber(svgElement),
+//                XmlDocParser.getElementColumnNumber(svgElement)));
+//      }
+//    }
+//  }
 
   void checkImageXlinkHrefInline(String svgDocEntry, Document doc)
   {

src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java

@@ -123,7 +123,6 @@ private void initialize()
     severities.put(MessageId.HTM_033, Severity.USAGE);
     severities.put(MessageId.HTM_036, Severity.SUPPRESSED);
     severities.put(MessageId.HTM_038, Severity.SUPPRESSED);
-    severities.put(MessageId.HTM_043, Severity.SUPPRESSED);
     severities.put(MessageId.HTM_044, Severity.USAGE);
     severities.put(MessageId.HTM_045, Severity.USAGE);
     severities.put(MessageId.HTM_046, Severity.ERROR);
@@ -134,8 +133,6 @@ private void initialize()
     severities.put(MessageId.HTM_051, Severity.WARNING);
     severities.put(MessageId.HTM_052, Severity.ERROR);
     severities.put(MessageId.HTM_053, Severity.INFO);
-    severities.put(MessageId.HTM_054, Severity.WARNING);
-    severities.put(MessageId.HTM_055, Severity.WARNING);
 
     // Media
     severities.put(MessageId.MED_001, Severity.ERROR);

src/main/java/com/adobe/epubcheck/messages/MessageId.java

@@ -116,7 +116,6 @@ public enum MessageId implements Comparable<MessageId>
   HTM_033("HTM-033"),
   HTM_036("HTM-036"),
   HTM_038("HTM-038"),
-  HTM_043("HTM-043"),
   HTM_044("HTM-044"),
   HTM_045("HTM-045"),
   HTM_046("HTM-046"),
@@ -127,8 +126,6 @@ public enum MessageId implements Comparable<MessageId>
   HTM_051("HTM-051"),
   HTM_052("HTM-052"),
   HTM_053("HTM_053"),
-  HTM_054("HTM-054"),
-  HTM_055("HTM-055"),
 
   // Messages associated with media (images, audio and video)
   MED_001("MED-001"),

src/main/java/com/adobe/epubcheck/opf/ValidationContext.java

@@ -63,6 +63,11 @@ public final class ValidationContext
    * Used to open resource streams. Guaranteed non-null.
    */
   public final GenericResourceProvider resourceProvider;
+  /**
+   * The Package Document item for the validated resource. Is absent if there is
+   * no item representing this resource in the Package Document.
+   */
+  public final Optional<OPFItem> opfItem;
   /**
    * The OCF Package the resource being validated belongs to. Is absent for
    * single-file validations.
@@ -84,7 +89,7 @@ public final class ValidationContext
 
   private ValidationContext(String path, String mimeType, EPUBVersion version, EPUBProfile profile,
       Report report, Locale locale, FeatureReport featureReport,
-      GenericResourceProvider resourceProvider, Optional<OCFPackage> ocf,
+      GenericResourceProvider resourceProvider, Optional<OPFItem> opfItem, Optional<OCFPackage> ocf,
       Optional<XRefChecker> xrefChecker, Set<String> pubTypes, Set<Property> properties)
   {
     super();
@@ -96,6 +101,7 @@ private ValidationContext(String path, String mimeType, EPUBVersion version, EPU
     this.locale = locale;
     this.featureReport = featureReport;
     this.resourceProvider = resourceProvider;
+    this.opfItem = opfItem;
     this.ocf = ocf;
     this.xrefChecker = xrefChecker;
     this.pubTypes = pubTypes;
@@ -225,16 +231,18 @@ public ValidationContextBuilder addProperty(Property property)
 
     public ValidationContext build()
     {
+      path = Strings.nullToEmpty(path);
       resourceProvider = (resourceProvider == null && ocf != null) ? ocf : resourceProvider;
       checkNotNull(resourceProvider);
       checkNotNull(report);
       Locale locale = MoreObjects.firstNonNull(
           (report instanceof LocalizableReport) ? ((LocalizableReport) report).getLocale() : null,
           Locale.getDefault());
-      return new ValidationContext(Strings.nullToEmpty(path), Strings.nullToEmpty(mimeType),
+      return new ValidationContext(path, Strings.nullToEmpty(mimeType),
           version != null ? version : EPUBVersion.Unknown,
           profile != null ? profile : EPUBProfile.DEFAULT, report, locale,
           featureReport != null ? featureReport : new FeatureReport(), resourceProvider,
+          (xrefChecker != null) ? xrefChecker.getResource(path) : Optional.<OPFItem> absent(),
           Optional.fromNullable(ocf), Optional.fromNullable(xrefChecker),
           pubTypes != null ? ImmutableSet.copyOf(pubTypes) : ImmutableSet.<String> of(),
           properties.build());

src/main/java/com/adobe/epubcheck/ops/OPSHandler30.java

@@ -83,6 +83,7 @@ public class OPSHandler30 extends OPSHandler
   protected boolean inSvg = false;
   protected boolean inBody = false;
   protected boolean inRegionBasedNav = false;
+  protected boolean isOutermostSVGAlreadyProcessed = false;
   protected boolean hasAltorAnnotation = false;
   protected boolean hasTitle = false;
 
@@ -319,10 +320,9 @@ else if (name.equals("math"))
       inMathML = true;
       hasAltorAnnotation = (null != e.getAttribute("alttext"));
     }
-    else if (!context.mimeType.equals("image/svg+xml") && name.equals("svg"))
+    else if (name.equals("svg"))
     {
-      requiredProperties.add(ITEM_PROPERTIES.SVG);
-      processStartSvg(e);
+      processSVG(e);
     }
     else if (EpubConstants.EpubTypeNamespaceUri.equals(e.getNamespace()) && name.equals("switch"))
     {
@@ -636,25 +636,23 @@ protected void processObject(XMLElement e)
     }
   }
 
-  protected void processStartSvg(XMLElement e)
+  protected void processSVG(XMLElement e)
   {
     inSvg = true;
-    boolean foundXmlLang = false;
-    boolean foundLang = false;
-    for (int i = 0; i < e.getAttributeCount() && !foundLang && !foundXmlLang; ++i)
+    if (!context.mimeType.equals("image/svg+xml"))
     {
-      XMLAttribute a = e.getAttribute(i);
-      if ("lang".compareTo(a.getName()) == 0)
-      {
-        foundXmlLang = foundXmlLang
-            | (EpubConstants.XmlNamespaceUri.compareTo(a.getNamespace()) == 0);
-        foundLang = (EpubConstants.HtmlNamespaceUri.compareTo(a.getNamespace()) == 0);
-      }
+      requiredProperties.add(ITEM_PROPERTIES.SVG);
     }
-    if (!foundLang || !foundXmlLang)
+    else if (!isOutermostSVGAlreadyProcessed)
     {
-      report.message(MessageId.HTM_043,
-          EPUBLocation.create(path, parser.getLineNumber(), parser.getColumnNumber(), e.getName()));
+      isOutermostSVGAlreadyProcessed = true;
+      if (context.opfItem.isPresent() && context.opfItem.get().isFixedLayout()
+          && e.getAttribute("viewBox") == null)
+      {
+
+        report.message(MessageId.HTM_048,
+            EPUBLocation.create(path, parser.getLineNumber(), parser.getColumnNumber()));
+      }
     }
   }
 

src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties

@@ -104,7 +104,6 @@ HTM_033=HTML 'head' element does not have a 'title' child element.
 HTM_036=IFrames are highly discouraged.
 HTM_038=Ensure b, i, em, and strong elements are used in compliance with W3C HTML5 directives.
 HTM_038_SUG=CSS styles are usually more appropriate for italics or bold text.
-HTM_043=SVG elements should include an xml:lang and lang attributes.
 HTM_044=Namespace uri '%1$s' was included but not used.
 HTM_044_SUG=Remove unused Namespace URIs.
 HTM_045=Encountered empty href.
@@ -113,16 +112,13 @@ HTM_046=Fixed format item has no viewport defined.
 HTM_046_SUG=A viewport declaration is required for fixed format items.
 HTM_047=Html viewport is missing height and/or width.
 HTM_047_SUG=The viewport declaration must declare both width and height.
-HTM_048=SVG ViewBox and width and height are missing on fixed format document.
-HTM_048_SUG=A viewBox declaration or width and height are required for fixed format documents.
+HTM_048=SVG Fixed-Layout Documents must have a 'viewBox' attribute (on the outermost 'svg' element).
 HTM_049=Html element does not have an xmlns set to 'http://www.w3.org/1999/xhtml'.
 HTM_049_SUG=Add xmlns="http://www.w3.org/1999/xhtml" to the html element.
 HTM_050=Found epub:type="pagebreak" attribute in content document.
 HTM_051=Found Microdata semantic enrichments but no RDFa. EDUPUB recommends using RDFa Lite.
 HTM_052=The property 'region-based' is only allowed on nav elements in Data Navigation Documents.
 HTM_053=Found an external file link (file://) in file: '%1$s'.
-HTM_054=The use of a 'viewBox' attribute is recommended on SVG elements.
-HTM_055=When no `viewBox` attribute is present, the values of the 'height' and 'width' attributes should be specified in pixels.
 
 #media
 MED_001=Video poster must have core media image type.

src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java

@@ -977,15 +977,13 @@ public void testFXL_WithSVG_NoViewbox() {
 
   @Test
   public void testFXL_WithSVG_NoViewbox_WidthHeight(){
-    //Testing for Width/Height but no viewbox - re: Issue 902
-    Collections.addAll(expectedWarnings, MessageId.HTM_054);
+    expectedErrors.add(MessageId.HTM_048);
     testValidateDocument("invalid/fxl-svg-no-viewbox");
   }
 
   @Test
   public void testFXL_WithSVG_NoViewbox_WidthHeightInPercent(){
-    //Testing for Width/Height but no viewbox - re: Issue 902
-    Collections.addAll(expectedWarnings, MessageId.HTM_054, MessageId.HTM_055);
+    expectedErrors.add(MessageId.HTM_048);
     testValidateDocument("invalid/fxl-svg-no-viewbox-widthheight-in-percent");
   }
 

src/test/resources/com/adobe/epubcheck/test/opf/viewport_expected_results.json

@@ -73,9 +73,9 @@
     } ],
     "suggestion" : "The viewport declaration must declare both width and height."
   }, {
-    "ID" : "HTM-054",
-    "severity" : "WARNING",
-    "message" : "The use of a 'viewBox' attribute is recommended on SVG elements.",
+    "ID" : "HTM-048",
+    "severity" : "ERROR",
+    "message" : "SVG Fixed-Layout Documents must have a 'viewBox' attribute (on the outermost 'svg' element).",
     "additionalLocations" : 0,
     "locations" : [ {
       "path" : "OPS/page005.svg",
@@ -116,12 +116,12 @@
   "checker" : {
     "path" : "./test-classes/com/adobe/epubcheck/test/opf/viewport.epub",
     "filename" : "viewport.epub",
-    "checkerVersion" : "4.2.0-beta-SNAPSHOT",
-    "checkDate" : "02-24-2019 00:46:05",
-    "elapsedTime" : 5586,
+    "checkerVersion" : "4.2.0-rc-SNAPSHOT",
+    "checkDate" : "03-16-2019 01:28:00",
+    "elapsedTime" : 80,
     "nFatal" : 0,
-    "nError" : 2,
-    "nWarning" : 1,
+    "nError" : 3,
+    "nWarning" : 0,
     "nUsage" : 3
   },
   "publication" : {