274698 - Improve nsSVGLength.cpp

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

these are some changes that I'd like to make to this file

Comment 2

[tor]

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

> /* void newValueSpecifiedUnits (in unsigned short unitType, in float valueInSpecifiedUnits); */
>  NS_IMETHODIMP
> nsSVGLength::NewValueSpecifiedUnits(PRUint16 unitType, float valueInSpecifiedUnits)
> {
>+  // We assume unitType != mSpecifiedUnitType since the consumer should use
>+  // SetValueInSpecifiedUnits in that case.
> 
>-  bool observer_change = (unitType != mSpecifiedUnitType);

> /* void convertToSpecifiedUnits (in unsigned short unitType); */
> NS_IMETHODIMP
> nsSVGLength::ConvertToSpecifiedUnits(PRUint16 unitType)
> {
>-  if (!IsValidUnitType(unitType)) return NS_ERROR_FAILURE;
>-
>-  bool observer_change = (unitType != mSpecifiedUnitType);

I don't think these optimizations should be removed.

> float nsSVGLength::AxisLength()
> {
>-  if (d == 0.0f)
>-    d = 1e-20f;
>+  if (d == 0.0f) {
>+    NS_ASSERTION(d!=0.0f, "invalid axis length");
>+    d = 1.0f;
>+  }
>+
>   return d;
> }

Not comfortable with this, as zero is a valid axis length.

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: quite a different patch

Okay, this is what I'd like to do before trying to fix bz's bug. The utility
functions that were only called from one place I've moved inline which makes
for better error reporting.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: quite a different patch v2

let's try that again

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: now without MaybeAddAsObserver() in ctor

Comment 6

[Jonathan Watt [:jwatt]]

Checked in.

Checking in mozilla/content/svg/content/src/nsSVGLength.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGLength.cpp,v  <--  nsSVGLength.cpp
new revision: 1.19; previous revision: 1.18
done