From ced3c1509223fefdcb266435dd97fc432241fa90 Mon Sep 17 00:00:00 2001 From: Romain Deltour Date: Sat, 16 Mar 2019 00:47:14 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20revert=20FXL=20SVG=20rules=20to=20EPUB?= =?UTF-8?q?=203.0.1=E2=80=99s=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` field to the validator context (retrieved from the `XRefChecker` in the builder, or absent). See also w3c/publ-epub-revision#1236 This PR (partially) reverts commit 17f5eeedbbb845703a9821ce38a059a4fb392ffa --- .../com/adobe/epubcheck/ctc/EpubSVGCheck.java | 60 ++++++------------- .../epubcheck/messages/DefaultSeverities.java | 3 - .../adobe/epubcheck/messages/MessageId.java | 3 - .../epubcheck/opf/ValidationContext.java | 12 +++- .../com/adobe/epubcheck/ops/OPSHandler30.java | 32 +++++----- .../messages/MessageBundle.properties | 6 +- .../api/Epub30CheckExpandedTest.java | 6 +- .../test/opf/viewport_expected_results.json | 16 ++--- 8 files changed, 53 insertions(+), 85 deletions(-) diff --git a/src/main/java/com/adobe/epubcheck/ctc/EpubSVGCheck.java b/src/main/java/com/adobe/epubcheck/ctc/EpubSVGCheck.java index 249c47d9b..6ad510fe3 100644 --- a/src/main/java/com/adobe/epubcheck/ctc/EpubSVGCheck.java +++ b/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) { diff --git a/src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java b/src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java index 6042161e0..3ed096cc6 100644 --- a/src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java +++ b/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); diff --git a/src/main/java/com/adobe/epubcheck/messages/MessageId.java b/src/main/java/com/adobe/epubcheck/messages/MessageId.java index 17f5c0f94..ec3b814cd 100644 --- a/src/main/java/com/adobe/epubcheck/messages/MessageId.java +++ b/src/main/java/com/adobe/epubcheck/messages/MessageId.java @@ -116,7 +116,6 @@ public enum MessageId implements Comparable 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 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"), diff --git a/src/main/java/com/adobe/epubcheck/opf/ValidationContext.java b/src/main/java/com/adobe/epubcheck/opf/ValidationContext.java index dd9217ba5..5922e0429 100644 --- a/src/main/java/com/adobe/epubcheck/opf/ValidationContext.java +++ b/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; /** * 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 ocf, + GenericResourceProvider resourceProvider, Optional opfItem, Optional ocf, Optional xrefChecker, Set pubTypes, Set 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. absent(), Optional.fromNullable(ocf), Optional.fromNullable(xrefChecker), pubTypes != null ? ImmutableSet.copyOf(pubTypes) : ImmutableSet. of(), properties.build()); diff --git a/src/main/java/com/adobe/epubcheck/ops/OPSHandler30.java b/src/main/java/com/adobe/epubcheck/ops/OPSHandler30.java index 0b989f367..ac8a23a80 100644 --- a/src/main/java/com/adobe/epubcheck/ops/OPSHandler30.java +++ b/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())); + } } } diff --git a/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties b/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties index be8f52a86..1781e8b8c 100644 --- a/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties +++ b/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. diff --git a/src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java b/src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java index 07591c04a..229c599e9 100644 --- a/src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java +++ b/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"); } diff --git a/src/test/resources/com/adobe/epubcheck/test/opf/viewport_expected_results.json b/src/test/resources/com/adobe/epubcheck/test/opf/viewport_expected_results.json index 962e135d7..c5d4afc4b 100644 --- a/src/test/resources/com/adobe/epubcheck/test/opf/viewport_expected_results.json +++ b/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" : {