css: handle line-height in the base SVG implementation

The SVG 1.1 implementation of Batik does not handle the line-height property,
and uses a fixed value of 1 instead (in SVG 1.1 the line height is defined to
be 1). The SVG 1.2 implementation does manage that property but uses a default
of 1.1.

This commit unifies the handling of line-height so the CSS property can be used
by both the base (SVG1.1) and 1.2 implementations, and both use the same default
(1.1 was chosen due to accessibility concerns).

This unification is important for the support of the CSS units `lh` and `rlh`
and for the handling of foreign elements.

After this commit, images rendered with the SVG 1.1 implementation of Batik may
show a different line-height with respect to EchoSVG.

MIGRATING_FROM_BATIK.md

@@ -48,11 +48,12 @@ Batik:
 7) EchoSVG does not depend on `xml-apis-ext-1.3.04.jar` to provide the SVGOM and
    SMIL APIs (see [#15](https://github.com/css4j/echosvg/issues/15)).
    If `xml-apis-ext` happens to be in your modulepath, you could configure an
-   exclusion for it (in Maven or Gradle), then use `svgom-api` and `smil-api`
-   from the [Web APIs](https://github.com/css4j/web-apis) project instead. If
-   your project requires the old SAC api which is provided by `xml-apis-ext` as
-   well, you can use [`sac.jar`](https://mvnrepository.com/artifact/org.w3c.css/sac)
-   as a replacement.
+   exclusion for it (in Maven or Gradle), and let `svgom-api` and `smil-api`
+   (transitive dependencies of EchoSVG from the [Web APIs](https://github.com/css4j/web-apis)
+   project) do the job, as they are compatible with Batik. If your project
+   requires the old SAC api which is provided by `xml-apis-ext` as well, you can
+   use [`sac.jar`](https://mvnrepository.com/artifact/org.w3c.css/sac) as a
+   replacement.
 
 8) `EchoSVGSecurityManager` is deprecated for removal due to Security Manager
    API being deprecated for removal since Java 17. Similarly,
@@ -66,8 +67,13 @@ Batik:
 10) `SVGAnimationElementBridge.initializeAnimation()` gained a `BridgeContext`
    argument and now returns a `boolean`.
 
+### Upgrading to EchoSVG 2.0
+
 11) The old and deprecated `CSSValue` API was replaced by an API close to W3C's
    Typed OM. One of the consequences is that if your scripts call `getPresentationAttribute`
    on `SVGStylable` objects, `getPresentationAttributeValue` should be called
    instead, obtaining a `CSSStyleValue`. Note that modern SVGOM has neither of
    those methods.
+
+12) The line-height property now has a global default of 1.1, instead of the
+   mixture of 1.1 and 1 that Batik uses.

echosvg-bridge/src/main/java/io/sf/carte/echosvg/bridge/svg12/SVGFlowRootElementBridge.java

@@ -35,6 +35,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.w3c.css.om.unit.CSSUnit;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.events.Event;
@@ -60,9 +61,7 @@
 import io.sf.carte.echosvg.constants.XMLConstants;
 import io.sf.carte.echosvg.css.engine.CSSEngine;
 import io.sf.carte.echosvg.css.engine.SVGCSSEngine;
-import io.sf.carte.echosvg.css.engine.value.ComputedValue;
 import io.sf.carte.echosvg.css.engine.value.Value;
-import io.sf.carte.echosvg.css.engine.value.svg12.LineHeightValue;
 import io.sf.carte.echosvg.css.engine.value.svg12.SVG12ValueConstants;
 import io.sf.carte.echosvg.dom.AbstractNode;
 import io.sf.carte.echosvg.dom.events.NodeEventTarget;
@@ -825,7 +824,7 @@ protected void initCSSPropertyIndexes(Element e) {
 		marginLeftIndex = eng.getPropertyIndex(SVG12CSSConstants.CSS_MARGIN_LEFT_PROPERTY);
 		indentIndex = eng.getPropertyIndex(SVG12CSSConstants.CSS_INDENT_PROPERTY);
 		textAlignIndex = eng.getPropertyIndex(SVG12CSSConstants.CSS_TEXT_ALIGN_PROPERTY);
-		lineHeightIndex = eng.getPropertyIndex(CSSConstants.CSS_LINE_HEIGHT_PROPERTY);
+		lineHeightIndex = eng.getLineHeightIndex();
 	}
 
 	public BlockInfo makeBlockInfo(BridgeContext ctx, Element element) {
@@ -881,23 +880,22 @@ else if (s == CSSConstants.CSS_END_VALUE)
 		String ln = element.getLocalName();
 		boolean rgnBr;
 		rgnBr = ln.equals(SVG12Constants.SVG_FLOW_REGION_BREAK_TAG);
-		return new BlockInfo(top, right, bottom, left, indent, textAlign, lineHeight, fontList, fontAttrs, rgnBr);
+		return new BlockInfo(top, right, bottom, left, indent, textAlign, lineHeight, fontList, fontAttrs,
+				rgnBr);
 	}
 
-	protected float getLineHeight(BridgeContext ctx, Element element, float fontSize) {
-		if (lineHeightIndex == -1)
+	private float getLineHeight(BridgeContext ctx, Element element, float fontSize) {
+		if (lineHeightIndex == -1) {
 			initCSSPropertyIndexes(element);
+		}
 
 		Value v = CSSUtilities.getComputedStyle(element, lineHeightIndex);
-		if (v.getCssValueType() == CSSValue.CssType.KEYWORD || v.isIdentifier(CSSConstants.CSS_NORMAL_VALUE)) {
-			return fontSize * 1.1f;
-		}
 
 		float lineHeight = v.getFloatValue();
-		if (v instanceof ComputedValue)
-			v = ((ComputedValue) v).getComputedValue();
-		if ((v instanceof LineHeightValue) && ((LineHeightValue) v).getFontSizeRelative())
+		if (v.getUnitType() == CSSUnit.CSS_NUMBER) {
 			lineHeight *= fontSize;
+		}
+
 		return lineHeight;
 	}
 

echosvg-css/src/main/java/io/sf/carte/echosvg/css/engine/SVG12CSSEngine.java

@@ -25,7 +25,6 @@
 import io.sf.carte.echosvg.css.engine.value.ValueManager;
 import io.sf.carte.echosvg.css.engine.value.svg.OpacityManager;
 import io.sf.carte.echosvg.css.engine.value.svg.SVGColorManager;
-import io.sf.carte.echosvg.css.engine.value.svg12.LineHeightManager;
 import io.sf.carte.echosvg.css.engine.value.svg12.MarginLengthManager;
 import io.sf.carte.echosvg.css.engine.value.svg12.MarginShorthandManager;
 import io.sf.carte.echosvg.css.engine.value.svg12.TextAlignManager;
@@ -51,7 +50,6 @@ public class SVG12CSSEngine extends SVGCSSEngine {
 	 */
 	public SVG12CSSEngine(Document doc, ParsedURL uri, Parser p, CSSContext ctx) {
 		super(doc, uri, p, SVG_VALUE_MANAGERS, SVG_SHORTHAND_MANAGERS, ctx);
-		lineHeightIndex = LINE_HEIGHT_INDEX;
 	}
 
 	/**
@@ -67,13 +65,12 @@ public SVG12CSSEngine(Document doc, ParsedURL uri, Parser p, CSSContext ctx) {
 	public SVG12CSSEngine(Document doc, ParsedURL uri, Parser p, ValueManager[] vms, ShorthandManager[] sms,
 			CSSContext ctx) {
 		super(doc, uri, p, mergeArrays(SVG_VALUE_MANAGERS, vms), mergeArrays(SVG_SHORTHAND_MANAGERS, sms), ctx);
-		lineHeightIndex = LINE_HEIGHT_INDEX;
 	}
 
 	/**
 	 * The value managers for SVG.
 	 */
-	public static final ValueManager[] SVG_VALUE_MANAGERS = { new LineHeightManager(),
+	public static final ValueManager[] SVG_VALUE_MANAGERS = {
 			new MarginLengthManager(SVG12CSSConstants.CSS_INDENT_PROPERTY),
 			new MarginLengthManager(SVG12CSSConstants.CSS_MARGIN_BOTTOM_PROPERTY),
 			new MarginLengthManager(SVG12CSSConstants.CSS_MARGIN_LEFT_PROPERTY),
@@ -90,8 +87,7 @@ public SVG12CSSEngine(Document doc, ParsedURL uri, Parser p, ValueManager[] vms,
 	//
 	// The property indexes.
 	//
-	public static final int LINE_HEIGHT_INDEX = SVGCSSEngine.FINAL_INDEX + 1;
-	public static final int INDENT_INDEX = LINE_HEIGHT_INDEX + 1;
+	public static final int INDENT_INDEX = SVGCSSEngine.FINAL_INDEX + 1;
 	public static final int MARGIN_BOTTOM_INDEX = INDENT_INDEX + 1;
 	public static final int MARGIN_LEFT_INDEX = MARGIN_BOTTOM_INDEX + 1;
 	public static final int MARGIN_RIGHT_INDEX = MARGIN_LEFT_INDEX + 1;

echosvg-css/src/main/java/io/sf/carte/echosvg/css/engine/SVGCSSEngine.java

@@ -36,6 +36,7 @@
 import io.sf.carte.echosvg.css.engine.value.css2.FontStyleManager;
 import io.sf.carte.echosvg.css.engine.value.css2.FontVariantManager;
 import io.sf.carte.echosvg.css.engine.value.css2.FontWeightManager;
+import io.sf.carte.echosvg.css.engine.value.css2.LineHeightManager;
 import io.sf.carte.echosvg.css.engine.value.css2.OverflowManager;
 import io.sf.carte.echosvg.css.engine.value.css2.SrcManager;
 import io.sf.carte.echosvg.css.engine.value.css2.TextDecorationManager;
@@ -99,8 +100,7 @@ public class SVGCSSEngine extends CSSEngine {
 	public SVGCSSEngine(Document doc, ParsedURL uri, Parser p, CSSContext ctx) {
 		super(doc, uri, p, SVG_VALUE_MANAGERS, SVG_SHORTHAND_MANAGERS, null, null, "style", null, "class", true, null,
 				ctx);
-		// SVG defines line-height to be font-size.
-		lineHeightIndex = fontSizeIndex;
+		lineHeightIndex = LINE_HEIGHT_INDEX;
 	}
 
 	/**
@@ -117,17 +117,15 @@ public SVGCSSEngine(Document doc, ParsedURL uri, Parser p, ValueManager[] vms, S
 			CSSContext ctx) {
 		super(doc, uri, p, mergeArrays(SVG_VALUE_MANAGERS, vms), mergeArrays(SVG_SHORTHAND_MANAGERS, sms), null, null,
 				"style", null, "class", true, null, ctx);
-		// SVG defines line-height to be font-size.
-		lineHeightIndex = fontSizeIndex;
+		lineHeightIndex = LINE_HEIGHT_INDEX;
 	}
 
 	protected SVGCSSEngine(Document doc, ParsedURL uri, Parser p, ValueManager[] vms, ShorthandManager[] sms,
 			String[] pe, String sns, String sln, String cns, String cln, boolean hints, String hintsNS,
 			CSSContext ctx) {
 		super(doc, uri, p, mergeArrays(SVG_VALUE_MANAGERS, vms), mergeArrays(SVG_SHORTHAND_MANAGERS, sms), pe, sns, sln,
 				cns, cln, hints, hintsNS, ctx);
-		// SVG defines line-height to be font-size.
-		lineHeightIndex = fontSizeIndex;
+		lineHeightIndex = LINE_HEIGHT_INDEX;
 	}
 
 	/**
@@ -175,7 +173,7 @@ protected static ShorthandManager[] mergeArrays(ShorthandManager[] a1, Shorthand
 			new ImageRenderingManager(), new KerningManager(),
 			new SpacingManager(CSSConstants.CSS_LETTER_SPACING_PROPERTY),
 			new SVGColorManager(CSSConstants.CSS_LIGHTING_COLOR_PROPERTY, ValueConstants.WHITE_RGB_VALUE),
-			new MarkerManager(CSSConstants.CSS_MARKER_END_PROPERTY),
+			new LineHeightManager(), new MarkerManager(CSSConstants.CSS_MARKER_END_PROPERTY),
 
 			new MarkerManager(CSSConstants.CSS_MARKER_MID_PROPERTY),
 			new MarkerManager(CSSConstants.CSS_MARKER_START_PROPERTY), new MaskManager(),
@@ -244,7 +242,8 @@ protected static ShorthandManager[] mergeArrays(ShorthandManager[] a1, Shorthand
 	public static final int KERNING_INDEX = IMAGE_RENDERING_INDEX + 1;
 	public static final int LETTER_SPACING_INDEX = KERNING_INDEX + 1;
 	public static final int LIGHTING_COLOR_INDEX = LETTER_SPACING_INDEX + 1;
-	public static final int MARKER_END_INDEX = LIGHTING_COLOR_INDEX + 1;
+	public static final int LINE_HEIGHT_INDEX = LIGHTING_COLOR_INDEX + 1;
+	public static final int MARKER_END_INDEX = LINE_HEIGHT_INDEX + 1;
 
 	public static final int MARKER_MID_INDEX = MARKER_END_INDEX + 1;
 	public static final int MARKER_START_INDEX = MARKER_MID_INDEX + 1;

echosvg-css/src/main/java/io/sf/carte/echosvg/css/engine/value/AbstractValueManager.java

@@ -106,10 +106,16 @@ public Value computeValue(CSSStylableElement elt, String pseudo, CSSEngine engin
 		return value;
 	}
 
+	/**
+	 * Obtain the float value, making sure that it is a <number> or a <length>.
+	 * 
+	 * @param cv the value.
+	 * @return the float value
+	 */
 	protected float lengthValue(Value cv) {
 		short unit = cv.getUnitType();
 		if (!CSSUnit.isLengthUnitType(unit) && unit != CSSUnit.CSS_NUMBER) {
-			throw createDOMException(unit);
+			throw createDOMException(cv);
 		}
 		return cv.getFloatValue();
 	}
@@ -118,8 +124,8 @@ protected float lengthValue(Value cv) {
 	 * Creates an INVALID_ACCESS_ERR exception.
 	 * @param unit the unit.
 	 */
-	protected DOMException createDOMException(int unit) {
-		Object[] p = { unit };
+	protected DOMException createDOMException(Value cv) {
+		Object[] p = { cv.getCssText(), cv.getUnitType() };
 		String s = Messages.formatMessage("invalid.value.access", p);
 		return new DOMException(DOMException.INVALID_ACCESS_ERR, s);
 	}