improved css font size validation // refs #776 fixes #529

* add 'ex' as valid font-size length unit and 'inherit' as valid keyword * add CssParser UnitTest for invalid font-size value '1m' // refs #529 just make sure that '1m' is not reported as a valid length unit * report unsupported font-size values (Units and keywords) as ERROR changes CSS_020 from USAGE to ERROR * add non-relative font-size to ACC_014/ACC_016 USAGE message * add jUnit test with all supported and some invalid CSS font-size values * update other tests and test resources to match new message format and severity
w3c · Sep 29, 2017 · 25c0b37 · 25c0b37
1 parent 03291e2
commit 25c0b37
Show file tree

Hide file tree

Showing 23 changed files with 394 additions and 217 deletions.
diff --git a/src/main/java/com/adobe/epubcheck/ctc/css/EpubCSSCheckCSSHandler.java b/src/main/java/com/adobe/epubcheck/ctc/css/EpubCSSCheckCSSHandler.java
@@ -544,11 +544,19 @@ void CheckFontSize(CssGrammar.CssConstruct construct, CssGrammar.CssDeclaration
       {
         if (!isGlobalFixedFormat || hasIndividualFixedFormatDocuments)
         {
+          // report non-relative font-size keyword as ACC USAGE message
           String value = construct.toCssString().toLowerCase(Locale.ROOT);
-          if (("smaller".compareTo(value) != 0) && ("larger".compareTo(value) != 0) && ("inherit".compareTo(value) != 0))
+
+          // report not allowed font-size keyword as ERROR message
+          if (!isFontSize(construct))
           {
-            getReport().message(id, getCorrectedEPUBLocation(path, declaration.getLocation().getLine(), declaration.getLocation().getColumn(), declaration.toCssString()));
+            getReport().message(MessageId.CSS_020, getCorrectedEPUBLocation(path, declaration.getLocation().getLine(), declaration.getLocation().getColumn(), declaration.toCssString()), construct.toCssString());
           }
+          else if (("smaller".compareTo(value) != 0) && ("larger".compareTo(value) != 0) && ("inherit".compareTo(value) != 0))
+          {
+            getReport().message(id, getCorrectedEPUBLocation(path, declaration.getLocation().getLine(), declaration.getLocation().getColumn(), declaration.toCssString()), construct.toCssString());
+          }
+
         }
         break;
       }
@@ -559,11 +567,17 @@ void CheckFontSize(CssGrammar.CssConstruct construct, CssGrammar.CssDeclaration
           switch (quantity.getUnit())
           {
             case EMS:
+            case EXS:
             case REMS:
             case PERCENTAGE:
               break;
+            case LENGTH:
+              // report absolute font-size as ACC USAGE message
+              getReport().message(id, getCorrectedEPUBLocation(path, declaration.getLocation().getLine(), declaration.getLocation().getColumn(), declaration.toCssString()), construct.toCssString());
+              break;
             default:
-              getReport().message(id, getCorrectedEPUBLocation(path, declaration.getLocation().getLine(), declaration.getLocation().getColumn(), declaration.toCssString()));
+              // report unsupported font-size as ERROR message
+              getReport().message(MessageId.CSS_020, getCorrectedEPUBLocation(path, declaration.getLocation().getLine(), declaration.getLocation().getColumn(), declaration.toCssString()), construct.toCssString());
               break;
           }
         }
@@ -672,6 +686,7 @@ boolean isFontSize(CssGrammar.CssConstruct construct)
           "xx-large",
           "larger",
           "smaller",
+          "inherit"
       };
       fontSizes = getHashSetFromStrings(fontSizeStrings);
     }
@@ -688,6 +703,7 @@ boolean isFontSize(CssGrammar.CssConstruct construct)
       {
         case PERCENTAGE:
         case EMS:
+        case EXS:
         case LENGTH:
           return true;
       }

diff --git a/src/main/java/com/adobe/epubcheck/messages/MessageDictionary.java b/src/main/java/com/adobe/epubcheck/messages/MessageDictionary.java
@@ -112,7 +112,7 @@ static Map<MessageId, Severity> getDefaultSeverities()
       map.put(MessageId.CSS_016, Severity.SUPPRESSED);
       map.put(MessageId.CSS_017, Severity.WARNING);
       map.put(MessageId.CSS_019, Severity.WARNING);
-      map.put(MessageId.CSS_020, Severity.USAGE);
+      map.put(MessageId.CSS_020, Severity.ERROR);
       map.put(MessageId.CSS_021, Severity.USAGE);
       map.put(MessageId.CSS_022, Severity.USAGE);
       map.put(MessageId.CSS_023, Severity.USAGE);

diff --git a/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties b/src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties
@@ -15,11 +15,11 @@ ACC_011=SVG hyperlinks should have a human-readable title (using the 'title' chi
 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.
-ACC_014=Value of CSS property 'font-size' does not use a relative size.
+ACC_014=Value of CSS property 'font-size' does not use a relative size: '%1$s'
 ACC_014_SUG=Acceptable values include percentages, em multiples, 'larger', 'smaller', 'normal' or 'inherit'.
 ACC_015=Value of CSS property 'line-height' does not use a relative size.
 ACC_015_SUG=Acceptable values include percentages, numeric multiples, em multiples, 'normal', or 'inherit'.
-ACC_016=Value of CSS property 'font-size' should use a relative size.
+ACC_016=Value of CSS property 'font-size' should use a relative size: '%1$s'
 ACC_016_SUG=Acceptable values include percentages, em multiples, 'larger', 'smaller', 'normal' or 'inherit'.
 ACC_017=Value of CSS property 'line-height' does not use a relative size.
 ACC_017_SUG=Acceptable values include percentages, numeric multiples, em multiples, 'normal', or 'inherit'.

diff --git a/src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java b/src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java
@@ -263,7 +263,7 @@ public void testValidateEPUB30_CSSURLS_1()
   @Test
   public void testValidateEPUB30_CSSURLS_2()
   {
-    Collections.addAll(expectedErrors, MessageId.OPF_027);
+    Collections.addAll(expectedErrors, MessageId.OPF_027, MessageId.CSS_020, MessageId.CSS_020);
     // 'imgs/table_header_bg_uni.jpg': referenced resource missing in the
     // package
     testValidateDocument("invalid/lorem-css-urls-2/");
@@ -273,6 +273,7 @@ public void testValidateEPUB30_CSSURLS_2()
   public void testValidateEPUB30_CSSURLS_3()
   {
     Collections.addAll(expectedWarnings, MessageId.CSS_017);
+    Collections.addAll(expectedErrors, MessageId.CSS_020, MessageId.CSS_020);
     // 'imgs/table_header_bg_uni.jpg': referenced resource missing in the
     // package
     testValidateDocument("invalid/lorem-css-urls-3/");
@@ -1073,5 +1074,12 @@ public void testOutOfSpineRef()
     testValidateDocument("invalid/href-outofspine");
   }
 
+  @Test
+  public void testInvalidCssFontSizeValue()
+  {
+    Collections.addAll(expectedErrors, MessageId.CSS_020, MessageId.CSS_020, MessageId.CSS_020);
+    testValidateDocument("invalid/invalid-css-font-size-value");
+  }
+
 
 }
diff --git a/src/test/java/com/adobe/epubcheck/api/Epub30CheckTest.java b/src/test/java/com/adobe/epubcheck/api/Epub30CheckTest.java
@@ -183,7 +183,7 @@ public void testValidateEPUB30Issue203()
   public void testValidateEPUB30Issue221()
   {
     Collections.addAll(expectedErrors, MessageId.CSS_008, MessageId.RSC_007, MessageId.RSC_007,
-        MessageId.RSC_007, MessageId.RSC_007, MessageId.RSC_007, MessageId.RSC_007);
+        MessageId.RSC_007, MessageId.RSC_007, MessageId.RSC_007, MessageId.RSC_007, MessageId.CSS_020);
     // syntax error in css that should not mask font-face
     testValidateDocument("invalid/issue221.epub");
   }

diff --git a/src/test/java/org/idpf/epubcheck/util/css/CssParserTest.java b/src/test/java/org/idpf/epubcheck/util/css/CssParserTest.java
@@ -2,7 +2,9 @@
 
 import static org.idpf.epubcheck.util.css.CssTokenList.Filters.FILTER_NONE;
 import static org.idpf.epubcheck.util.css.CssTokenList.Filters.FILTER_S_CMNT;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.io.PrintStream;
@@ -156,6 +158,19 @@ public void testParser013() throws Exception {
 		HandlerImpl handler = checkBasics(exec(css7));		
 		assertEquals(0, handler.errors.size());
 	}
+
+  @Test
+  public void testParser014() throws Exception {
+    String s = "E { font-size: 1m; } ";
+    HandlerImpl handler = checkBasics(exec(s));   
+    assertEquals(1, handler.selectors.size());
+    assertEquals(1, handler.declarations.size());
+    assertEquals(1, handler.declarations.get(0).components.size());
+    for(CssConstruct cc : handler.declarations.get(0).components) {
+      assertTrue(cc instanceof CssQuantity);
+      assertTrue(((CssQuantity)cc).subType != CssQuantity.Unit.LENGTH); // '1m' is invalid length
+    }
+  }
 
 	@Test
 	public void testParserAtRule001() throws Exception {

diff --git a/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/META-INF/container.xml b/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/META-INF/container.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<container version="1.0" xmlns="urn:oasis:names:tc:opendocument:xmlns:container">
+   <rootfiles>
+      <rootfile full-path="OPS/package.opf" media-type="application/oebps-package+xml"/>
+   </rootfiles>
+</container>
diff --git a/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/content_001.xhtml b/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/content_001.xhtml
@@ -0,0 +1,10 @@
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:epub="http://www.idpf.org/2007/ops" xml:lang="en">
+<head>
+	<title>Minimal EPUB</title>
+	<link rel="stylesheet" href="stylesheet.css" />
+</head>
+<body>
+<h1>Loomings</h1>
+<p>Call me Ishmael.</p>
+</body>
+</html>
diff --git a/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/nav.xhtml b/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/nav.xhtml
@@ -0,0 +1,12 @@
+<html xmlns="http://www.w3.org/1999/xhtml" xmlns:epub="http://www.idpf.org/2007/ops" xml:lang="en">
+<head>
+<title>Minimal Nav</title>
+</head>
+<body>
+<nav epub:type="toc">
+<ol>
+<li><a href="content_001.xhtml">content 001</a></li>
+</ol>
+</nav>
+</body>
+</html>
diff --git a/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/package.opf b/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/package.opf
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<package xmlns="http://www.idpf.org/2007/opf" version="3.0" xml:lang="en" unique-identifier="q">
+<metadata xmlns:dc="http://purl.org/dc/elements/1.1/">
+  <dc:title id="title">Minimal EPUB 3.0</dc:title>
+  <dc:language>en</dc:language>
+  <dc:identifier id="q">NOID</dc:identifier>
+  <meta property="dcterms:modified">2017-06-14T00:00:01Z</meta>
+</metadata>
+<manifest>
+  <item id="content_001"  href="content_001.xhtml" media-type="application/xhtml+xml"/>
+  <item id="nav"  href="nav.xhtml" media-type="application/xhtml+xml" properties="nav"/>
+  <item id="css" href="stylesheet.css" media-type="text/css" />
+</manifest>
+<spine>
+  <itemref idref="content_001" />
+</spine>
+</package>
diff --git a/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/stylesheet.css b/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/OPS/stylesheet.css
@@ -0,0 +1,43 @@
+@charset "UTF-8";
+
+/* *************************************************************
+ * invalid font sizes
+ * *********************************************************** */
+
+p { font-size: 1m; }
+p { font-size: meter; }
+p { font-size: in; }
+
+
+
+/* *************************************************************
+ * valid font sizes
+ * - https://www.w3.org/TR/CSS21/fonts.html#propdef-font-size
+ * - https://www.w3.org/TR/CSS21/syndata.html#value-def-length
+ * *********************************************************** */
+
+/* inherit */
+p { font-size: inherit; }
+/* percentage */
+p { font-size: 120%; }
+/* length */
+p { font-size: 1em; }
+p { font-size: 1ex; }
+p { font-size: 1in; }
+p { font-size: 1cm; }
+p { font-size: 1mm; }
+p { font-size: 1pt; }
+p { font-size: 1pc; }
+p { font-size: 1px; }
+/* relative */
+p { font-size: larger; }
+p { font-size: smaller; }
+/* absolute */
+p { font-size: xx-small; }
+p { font-size: x-small; }
+p { font-size: small; }
+p { font-size: medium; }
+p { font-size: large; }
+p { font-size: x-large; }
+p { font-size: xx-large; }
+
diff --git a/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/mimetype b/src/test/resources/30/expanded/invalid/invalid-css-font-size-value/mimetype
@@ -0,0 +1 @@
+application/epub+zip
diff --git a/...sources/com/adobe/epubcheck/test/command_line/severity_overrideBadId_expected_results.txt b/...sources/com/adobe/epubcheck/test/command_line/severity_overrideBadId_expected_results.txt
@@ -18,35 +18,35 @@ USAGE(OPF-059): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/toc.nc
 USAGE(OPF-059): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/toc.ncx(-1,-1): Spine item has no NCX entry reference.
 USAGE(CSS-012): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/external_css.xhtml(6,62): Document links to multiple CSS files.
 USAGE(CSS-012): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/external_css.xhtml(7,66): Document links to multiple CSS files.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(2,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(7,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(2,5): Value of CSS property 'font-size' does not use a relative size: '14px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(7,5): Value of CSS property 'font-size' does not use a relative size: '12px'
 USAGE(ACC-015): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(7,5): Value of CSS property 'line-height' does not use a relative size.
 USAGE(CSS-021): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(11,5): Shorthand CSS property 'font-family' specifies an invalid System Font.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(15,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(19,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(27,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(15,5): Value of CSS property 'font-size' does not use a relative size: '18px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(19,5): Value of CSS property 'font-size' does not use a relative size: '18px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(27,5): Value of CSS property 'font-size' does not use a relative size: '12px'
 USAGE(ACC-015): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(27,5): Value of CSS property 'line-height' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(35,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(39,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(35,5): Value of CSS property 'font-size' does not use a relative size: 'x-large'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(39,5): Value of CSS property 'font-size' does not use a relative size: 'large'
 WARNING(CSS-017): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(70,5): CSS selector specifies absolute position.
 USAGE(CSS-013): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(79,5): CSS property is declared !Important.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(90,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(90,5): Value of CSS property 'font-size' does not use a relative size: 'large'
 USAGE(CSS-023): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(97,1): CSS selector specifies media query.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(99,9): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(8,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(14,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(23,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(32,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(38,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(51,5): Value of CSS property 'font-size' does not use a relative size.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(58,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style.css(99,9): Value of CSS property 'font-size' does not use a relative size: 'large'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(8,5): Value of CSS property 'font-size' does not use a relative size: '12px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(14,5): Value of CSS property 'font-size' does not use a relative size: '22px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(23,5): Value of CSS property 'font-size' does not use a relative size: '14px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(32,5): Value of CSS property 'font-size' does not use a relative size: '12px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(38,5): Value of CSS property 'font-size' does not use a relative size: '12px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(51,5): Value of CSS property 'font-size' does not use a relative size: '11px'
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/unused.css(58,5): Value of CSS property 'font-size' does not use a relative size: '10px'
 WARNING(CSS-017): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(2,5): CSS selector specifies absolute position.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(10,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(10,5): Value of CSS property 'font-size' does not use a relative size: '12px'
 USAGE(ACC-015): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(10,5): Value of CSS property 'line-height' does not use a relative size.
 USAGE(CSS-013): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(15,5): CSS property is declared !Important.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(27,5): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(27,5): Value of CSS property 'font-size' does not use a relative size: 'large'
 USAGE(CSS-023): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(33,1): CSS selector specifies media query.
-USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(35,9): Value of CSS property 'font-size' does not use a relative size.
+USAGE(ACC-014): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/cssStyles.css(35,9): Value of CSS property 'font-size' does not use a relative size: 'large'
 USAGE(CSS-013): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style_tag_css.xhtml(9,13): CSS property is declared !Important.
 WARNING(CSS-017): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style_tag_css.xhtml(17,13): CSS selector specifies absolute position.
 USAGE(CSS-023): ./com/adobe/epubcheck/test/command_line/severity.epub/OPS/style_tag_css.xhtml(20,9): CSS selector specifies media query.