Bug 10746 - SVG code calls fooBaseValue() many times when it should call foo()
Summary: SVG code calls fooBaseValue() many times when it should call foo()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P4 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-06 01:18 PDT by Eric Seidel (no email)
Modified: 2006-09-08 18:50 PDT (History)
0 users

See Also:


Attachments
remove bogus fooBaseValue() calls (71.75 KB, patch)
2006-09-08 01:45 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Include some related cleanup, while we're at it. (98.11 KB, patch)
2006-09-08 02:03 PDT, Eric Seidel (no email)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2006-09-06 01:18:50 PDT
SVG code calls fooBaseValue() many times when it should call foo()

Now that the old broken SVGAnimated* code is removed as part of bug 10490, it has become obvious how many times the SVG code uses the wrong value (base vs. anim) for its calculations.  Before base and anim values were actually stored separately.  Now they are stored together until animation is enabled, at which point their storage is separate, until the element stops animating again.

Someone needs to do a search for all uses of BaseValue() and make sure that those callsites really need access to the baseVal instead of the animVal.  There are basically only 3 sections of c++ code which should ever need access to the baseValues.  Those are:
1.  attribute parsing code (always should use setFooBaseValue())
2.  animation code (may need to use fooBaseValue() to retrieve the base value for animation-related calculations).
3.  SVG DOM bindings.  JSSVGAnimated* bindings will need to call fooBaseValue() and setFooBaseValue()
Comment 1 Eric Seidel (no email) 2006-09-06 01:20:08 PDT
For those who are familiar with the SVG spec, this should be a very easy fix.
Comment 2 Eric Seidel (no email) 2006-09-08 01:45:45 PDT
Created attachment 10450 [details]
remove bogus fooBaseValue() calls
Comment 3 Eric Seidel (no email) 2006-09-08 02:03:34 PDT
Created attachment 10453 [details]
Include some related cleanup, while we're at it.
Comment 4 Darin Adler 2006-09-08 09:44:43 PDT
Comment on attachment 10453 [details]
Include some related cleanup, while we're at it.

r=me