411146 - Percentage dimensions on SVG in a foreignObject are broken

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

Attachment: testcase

Percentage dimensions on SVG descendants of a foreignObject do not work correctly when the descendant <svg> has a percentage width/height. This is because we end up going one coordinate context too high. More specifically, when resolving the percentage dimensions for the ellipse inside the foreignObject in the testcase attached, we end up with a stack like this:

  nsSVGElement::GetOwnerSVGElement()
  nsSVGSVGElement::GetOwnerSVGElement()
  nsSVGElement::GetCtx()
  nsSVGSVGElement::GetLength()
  nsSVGLength2::GetAxisLength()
  nsSVGLength2::GetUnitScaleFactor()
  nsSVGLength2::GetAnimValue()
  nsSVGElement::GetAnimatedLengthValues()
  nsSVGEllipseElement::ConstructPath()

When nsSVGSVGElement::GetLength() is called on the <svg> descendant of the <foreignObject>, it calls GetCtx() to resolve its own percentage width/height. At this point we want GetCtx to return null so that we use mViewportWidth and mViewportHeight as set by nsSVGOuterSVGFrame. Unfortunately it returns the <svg> parent of the <foreignObject> and we end up resolving percentages against it.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Robert Longson [:longsonr]]

Comment on attachment 295918 [details] [diff] [review]
patch

>   // are _we_ the outermost SVG element? If yes, return nsnull, but don't fail
>-  nsCOMPtr<nsIDOMSVGSVGElement> SVGSVGElement = do_QueryInterface((nsIDOMSVGElement*)this);
>-  if (SVGSVGElement) return NS_OK;
>+  if (this->Tag() == nsGkAtoms::svg) {

remove this-> there is a prior comment which makes it clear enough what is going on.

r=longsonr with that.

Comment 3

[Jonathan Watt [:jwatt]]

Oops, yeah. Thanks.

Comment 4

[Jonathan Watt [:jwatt]]

Comment on attachment 295918 [details] [diff] [review]
patch

Requesting approval1.9. Low risk change to fix percentage dimensions on SVG in a foreignObject.

Comment 5

[Mike Schroepfer]

Comment on attachment 295918 [details] [diff] [review]
patch

a1.9+ Awesome - and thanks for the reftests

Comment 6

[Jonathan Watt [:jwatt]]

Checked in.