Skip to content

Commit

Permalink
bridge: more resilience against missing or incorrect shape attributes…
Browse files Browse the repository at this point in the history
… [SVG2]
  • Loading branch information
carlosame committed Jun 27, 2024
1 parent a11a953 commit dd39155
Show file tree
Hide file tree
Showing 14 changed files with 225 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
*/
public class SVGOMEllipseElement extends SVGGraphicsElement implements SVGEllipseElement {

private static final long serialVersionUID = 1L;
private static final long serialVersionUID = 2L;

/**
* Table mapping XML attribute names to TraitInformation objects.
Expand Down Expand Up @@ -63,12 +63,12 @@ public class SVGOMEllipseElement extends SVGGraphicsElement implements SVGEllips
/**
* The 'rx' attribute value.
*/
protected SVGOMAnimatedLength rx;
protected AbstractSVGAnimatedLength rx;

/**
* The 'ry' attribute value.
*/
protected SVGOMAnimatedLength ry;
protected AbstractSVGAnimatedLength ry;

/**
* Creates a new SVGOMEllipseElement object.
Expand Down Expand Up @@ -104,8 +104,52 @@ private void initializeLiveAttributes() {
AbstractSVGAnimatedLength.HORIZONTAL_LENGTH, false);
cy = createLiveAnimatedLength(null, SVG_CY_ATTRIBUTE, SVG_ELLIPSE_CY_DEFAULT_VALUE,
AbstractSVGAnimatedLength.VERTICAL_LENGTH, false);
rx = createLiveAnimatedLength(null, SVG_RX_ATTRIBUTE, null, AbstractSVGAnimatedLength.HORIZONTAL_LENGTH, true);
ry = createLiveAnimatedLength(null, SVG_RY_ATTRIBUTE, null, AbstractSVGAnimatedLength.VERTICAL_LENGTH, true);
rx = new AbstractSVGAnimatedLength(this, null, SVG_RX_ATTRIBUTE, AbstractSVGAnimatedLength.HORIZONTAL_LENGTH,
true) {
@Override
protected String getDefaultValue() {
String defval = getAttribute(SVG_RY_ATTRIBUTE).trim();
if (defval.isEmpty() || SVG_AUTO_VALUE.equalsIgnoreCase(defval)) {
defval = "0";
}
return defval;
}

@Override
protected void attrChanged() {
super.attrChanged();
AbstractSVGAnimatedLength ry = (AbstractSVGAnimatedLength) getRy();
if (isSpecified() && !ry.isSpecified()) {
ry.attrChanged();
}
}
};
ry = new AbstractSVGAnimatedLength(this, null, SVG_RY_ATTRIBUTE, AbstractSVGAnimatedLength.VERTICAL_LENGTH,
true) {
@Override
protected String getDefaultValue() {
String defval = getAttribute(SVG_RX_ATTRIBUTE).trim();
if (defval.isEmpty() || SVG_AUTO_VALUE.equalsIgnoreCase(defval)) {
defval = "0";
}
return defval;
}

@Override
protected void attrChanged() {
super.attrChanged();
AbstractSVGAnimatedLength rx = (AbstractSVGAnimatedLength) getRx();
if (isSpecified() && !rx.isSpecified()) {
rx.attrChanged();
}
}
};

liveAttributeValues.put(null, SVG_RX_ATTRIBUTE, rx);
liveAttributeValues.put(null, SVG_RY_ATTRIBUTE, ry);
AnimatedAttributeListener l = ((SVGOMDocument) ownerDocument).getAnimatedAttributeListener();
rx.addAnimatedAttributeListener(l);
ry.addAnimatedAttributeListener(l);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package io.sf.carte.echosvg.anim.dom;

import org.w3c.dom.Attr;
import org.w3c.dom.Node;
import org.w3c.dom.svg.SVGAnimatedLength;
import org.w3c.dom.svg.SVGRadialGradientElement;
Expand Down Expand Up @@ -117,22 +116,22 @@ private void initializeLiveAttributes() {
false) {
@Override
protected String getDefaultValue() {
Attr attr = getAttributeNodeNS(null, SVG_CX_ATTRIBUTE);
if (attr == null) {
return SVG_RADIAL_GRADIENT_CX_DEFAULT_VALUE;
String defval = getAttribute(SVG_CX_ATTRIBUTE).trim();
if (defval.isEmpty()) {
defval = SVG_RADIAL_GRADIENT_CX_DEFAULT_VALUE;
}
return attr.getValue();
return defval;
}
};
fy = new AbstractSVGAnimatedLength(this, null, SVG_FY_ATTRIBUTE, AbstractSVGAnimatedLength.VERTICAL_LENGTH,
false) {
@Override
protected String getDefaultValue() {
Attr attr = getAttributeNodeNS(null, SVG_CY_ATTRIBUTE);
if (attr == null) {
return SVG_RADIAL_GRADIENT_CY_DEFAULT_VALUE;
String defval = getAttribute(SVG_CY_ATTRIBUTE).trim();
if (defval.isEmpty()) {
defval = SVG_RADIAL_GRADIENT_CY_DEFAULT_VALUE;
}
return attr.getValue();
return defval;
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package io.sf.carte.echosvg.anim.dom;

import org.w3c.dom.Attr;
import org.w3c.dom.Node;
import org.w3c.dom.svg.SVGAnimatedLength;
import org.w3c.dom.svg.SVGRectElement;
Expand Down Expand Up @@ -126,11 +125,11 @@ private void initializeLiveAttributes() {
true) {
@Override
protected String getDefaultValue() {
Attr attr = getAttributeNodeNS(null, SVG_RY_ATTRIBUTE);
if (attr == null) {
return "0";
String defval = getAttribute(SVG_RY_ATTRIBUTE).trim();
if (defval.isEmpty() || SVG_AUTO_VALUE.equalsIgnoreCase(defval)) {
defval = "0";
}
return attr.getValue();
return defval;
}

@Override
Expand All @@ -146,11 +145,11 @@ protected void attrChanged() {
true) {
@Override
protected String getDefaultValue() {
Attr attr = getAttributeNodeNS(null, SVG_RX_ATTRIBUTE);
if (attr == null) {
return "0";
String defval = getAttribute(SVG_RX_ATTRIBUTE).trim();
if (defval.isEmpty() || SVG_AUTO_VALUE.equalsIgnoreCase(defval)) {
defval = "0";
}
return attr.getValue();
return defval;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,29 +75,19 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {

// 'cx' attribute - default is 0
AbstractSVGAnimatedLength _cx = (AbstractSVGAnimatedLength) ce.getCx();
float cx = _cx.getCheckedValue();
float cx = safeAnimatedCheckedValue(_cx, 0f);

// 'cy' attribute - default is 0
AbstractSVGAnimatedLength _cy = (AbstractSVGAnimatedLength) ce.getCy();
float cy = _cy.getCheckedValue();
float cy = safeAnimatedCheckedValue(_cy, 0f);

// 'r' attribute - default is 0 (SVG2)
AbstractSVGAnimatedLength _r = (AbstractSVGAnimatedLength) ce.getR();
float r;
try {
r = _r.getCheckedValue();
} catch (LiveAttributeException ex) {
r = 0f;
BridgeException be = new BridgeException(ctx, ex);
if (ctx.userAgent == null) {
throw be;
}
ctx.userAgent.displayError(be);
}
float r = safeAnimatedCheckedValue(_r, 0f);

float x = cx - r;
float y = cy - r;
float w = r * 2;
float w = r * 2f;
shapeNode.setShape(new Ellipse2D.Float(x, y, w, w));
} catch (LiveAttributeException ex) {
throw new BridgeException(ctx, ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {

// 'cx' attribute - default is 0
AbstractSVGAnimatedLength _cx = (AbstractSVGAnimatedLength) ee.getCx();
float cx = _cx.getCheckedValue();
float cx = safeAnimatedCheckedValue(_cx, 0f);

// 'cy' attribute - default is 0
AbstractSVGAnimatedLength _cy = (AbstractSVGAnimatedLength) ee.getCy();
float cy = _cy.getCheckedValue();
float cy = safeAnimatedCheckedValue(_cy, 0f);

// 'rx' attribute - default is auto (SVG2)
boolean rxAuto = false;
Expand Down Expand Up @@ -121,7 +121,7 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {
rx = ry;
}

shapeNode.setShape(new Ellipse2D.Float(cx - rx, cy - ry, rx * 2, ry * 2));
shapeNode.setShape(new Ellipse2D.Float(cx - rx, cy - ry, rx * 2f, ry * 2f));
} catch (LiveAttributeException ex) {
throw new BridgeException(ctx, ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,39 +77,19 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {

// 'x' attribute - default is 0
AbstractSVGAnimatedLength _x = (AbstractSVGAnimatedLength) re.getX();
float x = _x.getCheckedValue();
float x = safeAnimatedCheckedValue(_x, 0f);

// 'y' attribute - default is 0
AbstractSVGAnimatedLength _y = (AbstractSVGAnimatedLength) re.getY();
float y = _y.getCheckedValue();
float y = safeAnimatedCheckedValue(_y, 0f);

// 'width' attribute - required
// 'width' attribute - default is 0
AbstractSVGAnimatedLength _width = (AbstractSVGAnimatedLength) re.getWidth();
float w;
try {
w = _width.getCheckedValue();
} catch (LiveAttributeException ex) {
w = 0;
BridgeException be = new BridgeException(ctx, ex);
if (ctx.userAgent == null) {
throw be;
}
ctx.userAgent.displayError(be);
}
float w = safeAnimatedCheckedValue(_width, 0f);

// 'height' attribute - required
// 'height' attribute - default is 0
AbstractSVGAnimatedLength _height = (AbstractSVGAnimatedLength) re.getHeight();
float h;
try {
h = _height.getCheckedValue();
} catch (LiveAttributeException ex) {
h = 0;
BridgeException be = new BridgeException(ctx, ex);
if (ctx.userAgent == null) {
throw be;
}
ctx.userAgent.displayError(be);
}
float h = safeAnimatedCheckedValue(_height, 0f);

// 'rx' attribute - default is 0
boolean rxAuto = false;
Expand All @@ -118,16 +98,16 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {
try {
rx = _rx.getCheckedValue();
} catch (LiveAttributeException ex) {
rx = 0;
rx = 0f;
rxAuto = true;
BridgeException be = new BridgeException(ctx, ex);
if (ctx.userAgent == null) {
throw be;
}
ctx.userAgent.displayError(be);
}
if (rx > w / 2) {
rx = w / 2;
if (rx > w / 2f) {
rx = w / 2f;
}

// 'ry' attribute - default is rx
Expand All @@ -143,8 +123,8 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {
}
ctx.userAgent.displayError(be);
}
if (ry > h / 2) {
ry = h / 2;
if (ry > h / 2f) {
ry = h / 2f;
}

// Check whether rx was auto
Expand All @@ -158,10 +138,10 @@ protected void buildShape(BridgeContext ctx, Element e, ShapeNode shapeNode) {
}

Shape shape;
if (rx == 0 || ry == 0) {
if (rx == 0f || ry == 0f) {
shape = new Rectangle2D.Float(x, y, w, h);
} else {
shape = new RoundRectangle2D.Float(x, y, w, h, rx * 2, ry * 2);
shape = new RoundRectangle2D.Float(x, y, w, h, rx * 2f, ry * 2f);
}
shapeNode.setShape(shape);
} catch (LiveAttributeException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@

import org.w3c.dom.Element;

import io.sf.carte.echosvg.anim.dom.AbstractSVGAnimatedLength;
import io.sf.carte.echosvg.css.engine.CSSEngineEvent;
import io.sf.carte.echosvg.css.engine.SVGCSSEngine;
import io.sf.carte.echosvg.dom.svg.LiveAttributeException;
import io.sf.carte.echosvg.gvt.GraphicsNode;
import io.sf.carte.echosvg.gvt.ShapeNode;
import io.sf.carte.echosvg.gvt.ShapePainter;
Expand Down Expand Up @@ -129,6 +131,28 @@ protected ShapePainter createShapePainter(BridgeContext ctx, Element e, ShapeNod
*/
protected abstract void buildShape(BridgeContext ctx, Element e, ShapeNode node);

/**
* Give a safe value for an animated length, regardless of exceptions.
*
* @param animValue the animated length.
* @param defValue the default value.
* @return the value.
*/
float safeAnimatedCheckedValue(AbstractSVGAnimatedLength animValue, float defValue) {
float value;
try {
value = animValue.getCheckedValue();
} catch (LiveAttributeException ex) {
value = defValue;
BridgeException be = new BridgeException(ctx, ex);
if (ctx.userAgent == null) {
throw be;
}
ctx.userAgent.displayError(be);
}
return value;
}

/**
* Returns false as shapes are not a container.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

import io.sf.carte.echosvg.anim.dom.AbstractSVGAnimatedLength;
import io.sf.carte.echosvg.anim.dom.AnimatedLiveAttributeValue;
import io.sf.carte.echosvg.anim.dom.SVGOMAnimatedLength;
import io.sf.carte.echosvg.anim.dom.SVGOMDocument;
import io.sf.carte.echosvg.anim.dom.SVGOMUseElement;
import io.sf.carte.echosvg.constants.XMLConstants;
Expand Down Expand Up @@ -173,11 +172,11 @@ public CompositeGraphicsNode buildCompositeGraphicsNode(BridgeContext ctx, Eleme
// on the 'use' element, then these values will override the
// corresponding attributes on the 'svg' in the generated tree.
try {
SVGOMAnimatedLength al = (SVGOMAnimatedLength) ue.getWidth();
AbstractSVGAnimatedLength al = (AbstractSVGAnimatedLength) ue.getWidth();
if (al.isSpecified()) {
localRefElement.setAttributeNS(null, SVG_WIDTH_ATTRIBUTE, al.getAnimVal().getValueAsString());
}
al = (SVGOMAnimatedLength) ue.getHeight();
al = (AbstractSVGAnimatedLength) ue.getHeight();
if (al.isSpecified()) {
localRefElement.setAttributeNS(null, SVG_HEIGHT_ATTRIBUTE, al.getAnimVal().getValueAsString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,14 @@ public void testBridgeExceptionInvalidCSS() throws Exception {
}

@Test
public void testBridgeExceptionEllipseMissingRx() throws Exception {
runTest("application/java-archive", "bridge/error/ellipse-missing-rx", "ANY", false, false,
public void testBridgeExceptionEllipseWrongUnitRx() throws Exception {
runTest("text/javascript", "bridge/error/ellipse-wrong-unit-rx", "ANY", false, false,
"io.sf.carte.echosvg.bridge.BridgeException");
}

@Test
public void testBridgeExceptionEllipseMissingRy() throws Exception {
runTest("application/java-archive", "bridge/error/ellipse-missing-ry", "ANY", false, false,
public void testBridgeExceptionEllipseWrongUnitRy() throws Exception {
runTest("text/javascript", "bridge/error/ellipse-wrong-unit-ry", "ANY", false, false,
"io.sf.carte.echosvg.bridge.BridgeException");
}

Expand Down
Loading

0 comments on commit dd39155

Please sign in to comment.