Follow-up to previous commit: do not compare CSS values, except `inhe…

…rit`, by reference

echosvg-bridge/src/main/java/io/sf/carte/echosvg/bridge/CSSFontFace.java

@@ -28,9 +28,9 @@
 import io.sf.carte.echosvg.css.engine.SVGCSSEngine;
 import io.sf.carte.echosvg.css.engine.StyleMap;
 import io.sf.carte.echosvg.css.engine.value.Value;
-import io.sf.carte.echosvg.css.engine.value.ValueConstants;
 import io.sf.carte.echosvg.css.engine.value.ValueManager;
 import io.sf.carte.echosvg.gvt.font.GVTFontFamily;
+import io.sf.carte.echosvg.util.CSSConstants;
 import io.sf.carte.echosvg.util.ParsedURL;
 import io.sf.carte.echosvg.util.SVGConstants;
 
@@ -83,7 +83,7 @@ public static CSSFontFace createCSSFontFace(CSSEngine eng, FontFaceRule ffr) {
 		v = sm.getValue(SVGCSSEngine.SRC_INDEX);
 
 		ParsedURL base = ffr.getURL();
-		if ((v != null) && (v != ValueConstants.NONE_VALUE)) {
+		if (v != null && !v.isIdentifier(CSSConstants.CSS_NONE_VALUE)) {
 			if (v.getCssValueType() == CssType.TYPED) {
 				ret.srcs = new LinkedList<>();
 				ret.srcs.add(getSrcValue(v, base));

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

@@ -56,12 +56,12 @@
 import io.sf.carte.echosvg.bridge.TextUtilities;
 import io.sf.carte.echosvg.bridge.UserAgent;
 import io.sf.carte.echosvg.constants.XMLConstants;
+import io.sf.carte.echosvg.css.dom.CSSValue.Type;
 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.ValueConstants;
-import io.sf.carte.echosvg.css.engine.value.svg.SVGValueConstants;
 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;
@@ -851,20 +851,25 @@ public BlockInfo makeBlockInfo(BridgeContext ctx, Element element) {
 		v = CSSUtilities.getComputedStyle(element, textAlignIndex);
 		if (v == ValueConstants.INHERIT_VALUE) {
 			v = CSSUtilities.getComputedStyle(element, SVGCSSEngine.DIRECTION_INDEX);
-			if (v == ValueConstants.LTR_VALUE)
+			if (v.isIdentifier(CSSConstants.CSS_LTR_VALUE))
 				v = SVG12ValueConstants.START_VALUE;
 			else
 				v = SVG12ValueConstants.END_VALUE;
 		}
 		int textAlign;
-		if (v == SVG12ValueConstants.START_VALUE)
-			textAlign = BlockInfo.ALIGN_START;
-		else if (v == SVG12ValueConstants.CENTER_VALUE || v == SVGValueConstants.MIDDLE_VALUE)
-			textAlign = BlockInfo.ALIGN_MIDDLE;
-		else if (v == SVG12ValueConstants.END_VALUE)
-			textAlign = BlockInfo.ALIGN_END;
-		else
+		if (v != null && v.getPrimitiveType() == Type.IDENT) {
+			String s = v.getIdentifierValue();
+			if (s == CSSConstants.CSS_START_VALUE)
+				textAlign = BlockInfo.ALIGN_START;
+			else if (s == CSSConstants.CSS_CENTER_VALUE || s == CSSConstants.CSS_MIDDLE_VALUE)
+				textAlign = BlockInfo.ALIGN_MIDDLE;
+			else if (s == CSSConstants.CSS_END_VALUE)
+				textAlign = BlockInfo.ALIGN_END;
+			else
+				textAlign = BlockInfo.ALIGN_FULL;
+		} else {
 			textAlign = BlockInfo.ALIGN_FULL;
+		}
 
 		Map<Attribute, Object> fontAttrs = new HashMap<>(20);
 		List<GVTFont> fontList = getFontList(ctx, element, fontAttrs);
@@ -883,7 +888,7 @@ protected float getLineHeight(BridgeContext ctx, Element element, float fontSize
 			initCSSPropertyIndexes(element);
 
 		Value v = CSSUtilities.getComputedStyle(element, lineHeightIndex);
-		if ((v == ValueConstants.INHERIT_VALUE) || (v == SVG12ValueConstants.NORMAL_VALUE)) {
+		if (v == ValueConstants.INHERIT_VALUE || v.isIdentifier(CSSConstants.CSS_NORMAL_VALUE)) {
 			return fontSize * 1.1f;
 		}
 

echosvg-css/src/main/java/io/sf/carte/echosvg/css/dom/CSSOMValue.java

@@ -421,6 +421,11 @@ public String getIdentifier() {
 		return valueProvider.getValue().getIdentifierValue();
 	}
 
+	@Override
+	public Value clone() {
+		return valueProvider.getValue().clone();
+	}
+
 	/**
 	 * To provide the actual value.
 	 */
@@ -433,63 +438,4 @@ public interface ValueProvider {
 
 	}
 
-	/**
-	 * To store a component.
-	 */
-	protected abstract class AbstractComponent implements Value {
-
-		/**
-		 * The returns the actual value of this component.
-		 */
-		protected abstract Value getValue();
-
-		@Override
-		public String getCssText() {
-			return valueProvider.getValue().getCssText();
-		}
-
-		@Override
-		public CssType getCssValueType() {
-			return valueProvider.getValue().getCssValueType();
-		}
-
-		@Override
-		public Type getPrimitiveType() {
-			return valueProvider.getValue().getPrimitiveType();
-		}
-
-		@Override
-		public float getFloatValue() throws DOMException {
-			return valueProvider.getValue().getFloatValue();
-		}
-
-		@Override
-		public String getStringValue() throws DOMException {
-			return valueProvider.getValue().getStringValue();
-		}
-
-		@Override
-		public CSSCounterValue getCounterValue() throws DOMException {
-			throw new DOMException(DOMException.INVALID_ACCESS_ERR, "");
-		}
-
-		@Override
-		public RectValue getRectValue() throws DOMException {
-			throw new DOMException(DOMException.INVALID_ACCESS_ERR, "");
-		}
-
-		@Override
-		public ColorValue getColorValue() throws DOMException {
-			throw new DOMException(DOMException.INVALID_ACCESS_ERR, "");
-		}
-
-		// CSSStyleValueList ///////////////////////////////////////////////////
-
-		@Override
-		public int getLength() {
-			throw new DOMException(DOMException.INVALID_ACCESS_ERR, "");
-		}
-
-	}
-
 }

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

@@ -167,7 +167,8 @@ public Value createValue(LexicalUnit lunit, CSSEngine engine) throws DOMExceptio
 		case RGBCOLOR:
 			return createRGBColor(lunit);
 		default:
-			return super.createValue(lunit, engine);
+			// Clone so colors can be modified
+			return super.createValue(lunit, engine).clone();
 		}
 	}
 

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

@@ -53,7 +53,7 @@ public int hashCode() {
 	 */
 	@Override
 	public boolean equals(Object obj) {
-		if (obj == null || !(obj instanceof AbstractStringValue)
+		if (!(obj instanceof AbstractStringValue)
 				|| getPrimitiveType() != ((AbstractStringValue) obj).getPrimitiveType()) {
 			return false;
 		}

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

@@ -63,10 +63,18 @@ public AbstractValueList(char s, int initialCapacity) {
 		items = new ArrayList<>(initialCapacity);
 	}
 
+	/**
+	 * Returns a deep copy of this instance.
+	 * 
+	 * @return a copy of this instance.
+	 */
+	@SuppressWarnings("unchecked")
 	@Override
 	public AbstractValueList<V> clone() {
 		AbstractValueList<V> clon = new AbstractValueList<>(separator, items.size());
-		clon.items.addAll(items);
+		for (V item : items) {
+			clon.items.add((V) item.clone());
+		}
 		return clon;
 	}
 

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

@@ -110,7 +110,7 @@ public void setColorSpace(String cs) {
 	}
 
 	static String canonicalName(String cs) throws DOMException {
-		cs = cs.toLowerCase(Locale.ROOT);
+		cs = cs.toLowerCase(Locale.ROOT).intern();
 		if (!predefinedSpaces.contains(cs)) {
 			throw new DOMException(DOMException.NOT_SUPPORTED_ERR,
 					"Unsupported color space: " + cs);

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

@@ -18,6 +18,7 @@
  */
 package io.sf.carte.echosvg.css.engine.value;
 
+import org.w3c.api.DOMTypeException;
 import org.w3c.css.om.typed.CSSKeywordValue;
 
 import io.sf.carte.doc.style.css.parser.ParseHelper;
@@ -32,6 +33,8 @@ public class IdentValue extends AbstractStringValue implements CSSKeywordValue {
 
 	/**
 	 * Creates a new IdentValue.
+	 * 
+	 * @param s an interned identifier string.
 	 */
 	public IdentValue(String s) {
 		super(s);
@@ -47,7 +50,7 @@ public IdentValue(String s) {
 	 * @return an immutable identifier value.
 	 */
 	public static IdentValue createConstant(String s) {
-		return new ImmutableIdentValue(s);
+		return new ImmutableIdentValue(s.intern());
 	}
 
 	/**
@@ -58,20 +61,6 @@ public Type getPrimitiveType() {
 		return Type.IDENT;
 	}
 
-	/**
-	 * Indicates whether some other object is "equal to" this one.
-	 * 
-	 * @param obj the reference object with which to compare.
-	 */
-	@Override
-	public boolean equals(Object obj) {
-		if (obj == null || !(obj instanceof Value)) {
-			return false;
-		}
-		Value v = (Value) obj;
-		return v.getPrimitiveType() == Type.IDENT && value.equals(v.getIdentifierValue());
-	}
-
 	/**
 	 * A string representation of the current value.
 	 */
@@ -90,6 +79,23 @@ public String getStringValue() {
 		return value;
 	}
 
+	@Override
+	public void setValue(String value) throws DOMTypeException {
+		if (value == null || (value = value.trim()).isEmpty()) {
+			throw new DOMTypeException("Value is null or empty.");
+		}
+		this.value = value.intern();
+
+		if (handler != null) {
+			handler.valueChanged(this);
+		}
+	}
+
+	@Override
+	public boolean isIdentifier(String internedIdent) {
+		return value == internedIdent;
+	}
+
 	@Override
 	public IdentValue clone() {
 		return new IdentValue(value);

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

@@ -31,6 +31,8 @@ class ImmutableIdentValue extends IdentValue {
 
 	/**
 	 * Creates a new immutable IdentValue.
+	 * 
+	 * @param s an interned identifier string.
 	 */
 	public ImmutableIdentValue(String s) {
 		super(s);

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

@@ -91,6 +91,16 @@ public void setB(CSSNumericValue b) throws DOMSyntaxException {
 		immutable();
 	}
 
+	@Override
+	public void setAlpha(double alpha) {
+		immutable();
+	}
+
+	@Override
+	public void setAlpha(CSSNumericValue alpha) throws DOMSyntaxException {
+		immutable();
+	}
+
 	@Override
 	public RGBColorValue clone() {
 		RGBColorValue clon;

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

@@ -85,17 +85,7 @@ static FloatValue createLength(LexicalUnit lu) {
 	 */
 	@Override
 	public Value createFloatValue(short type, float floatValue) throws DOMException {
-		switch (type) {
-		case CSSUnit.CSS_PERCENTAGE:
-		case CSSUnit.CSS_EM:
-		case CSSUnit.CSS_EX:
-		case CSSUnit.CSS_PX:
-		case CSSUnit.CSS_CM:
-		case CSSUnit.CSS_MM:
-		case CSSUnit.CSS_IN:
-		case CSSUnit.CSS_PT:
-		case CSSUnit.CSS_PC:
-		case CSSUnit.CSS_NUMBER:
+		if (type == CSSUnit.CSS_NUMBER || type == CSSUnit.CSS_PERCENTAGE || CSSUnit.isLengthUnitType(type)) {
 			return new FloatValue(type, floatValue);
 		}
 		throw createInvalidFloatTypeDOMException(type);

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

@@ -58,20 +58,6 @@ public String getURIValue() {
 		return value;
 	}
 
-	/**
-	 * Indicates whether some other object is "equal to" this one.
-	 * 
-	 * @param obj the reference object with which to compare.
-	 */
-	@Override
-	public boolean equals(Object obj) {
-		if (obj == null || !(obj instanceof Value)) {
-			return false;
-		}
-		Value v = (Value) obj;
-		return v.getPrimitiveType() == Type.STRING && value.equals(v.getStringValue());
-	}
-
 	/**
 	 * A string representation of the current value.
 	 */

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

@@ -54,20 +54,6 @@ public String getURIValue() {
 		return value;
 	}
 
-	/**
-	 * Indicates whether some other object is "equal to" this one.
-	 * 
-	 * @param obj the reference object with which to compare.
-	 */
-	@Override
-	public boolean equals(Object obj) {
-		if (obj == null || !(obj instanceof Value)) {
-			return false;
-		}
-		Value v = (Value) obj;
-		return v.getPrimitiveType() == Type.URI && value.equals(v.getURIValue());
-	}
-
 	/**
 	 * A string representation of the current value.
 	 */

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

@@ -112,4 +112,24 @@ default boolean isComponent() {
 		return false;
 	}
 
+	/**
+	 * Do this value represent the given identifier?
+	 * 
+	 * @param internedIdent the interned identifier string.
+	 * @return {@code true} if the value is a component.
+	 */
+	default boolean isIdentifier(String internedIdent) {
+		return false;
+	}
+
+	/**
+	 * Create and return a copy of this object.
+	 * <p>
+	 * If this object is unmodifiable, the clone will be modifiable.
+	 * </p>
+	 * 
+	 * @return a modifiable copy of this object.
+	 */
+	Value clone();
+
 }