[Webkit-unassigned] [Bug 26019] beginElement broken by setAttribute

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26019


Dinesh Garg <dinesh.garg at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #67271|0                           |1
        is obsolete|                            |
  Attachment #69401|                            |review?, commit-queue?
               Flag|                            |




--- Comment #8 from Dinesh Garg <dinesh.garg at gmail.com>  2010-09-30 17:21:22 PST ---
Created an attachment (id=69401)
 --> (https://bugs.webkit.org/attachment.cgi?id=69401)
Propose fix after review comments

(In reply to comment #7)
> > +        New test case for BugId - 26019:
> You can omit this line.
Done

> > +   var animatedElement;
> > +   animatedElement = document.getElementById("dTripper");
> > +   animatedElement.setAttribute("to", 420);
> > +   animatedElement.beginElement();
> > +   setTimeout(beginElement,100);
> I don't understand, why you are doing setAttribute("to".., then calling beginElement, and then setTimeout(beginElement, 100) which does the same? Please explain why this is needed.

I am removing setAttribute from here. setAttribute() breaks beginElementAt() when beginElementAt() has been called. i.e. 
beginElementAt() - setAttribute() - beginElementAt() is broken but not setAttribute() - beginElementAt()

> Also you shouldn't use 100ms as timeout, try setTImeout(beginElement, 0) if you want it async.
I want some timegap after beginElementAt to be sure that Animation has started.

> > +   function beginElement()
> > +   {
> > +        animatedElement.setAttribute("to", 420);
> > +        animatedElement.beginElement();
> > +        setTimeout(dumpResult,100);
> Same here, just use setTimeout(dumpResult, 0)
I want circle to move so that I can compare circle positions i.e. current  and last one. Value 0 wont help me. if bug fix is working circle will move next time beginElementAt() is called and then dumpresult shall compare current and positions and dump result accordingly.

> > +        beginElement broken by setAttribute
> > +        https://bugs.webkit.org/show_bug.cgi?id=26019
> Add a newline here.
Done

> > +        Initialized animation element state in beginElementAt
> The why is missing here, please provide a more detailed ChangeLog, another sentence describing why it is needed etc.
Done

> > @@ -556,7 +558,7 @@ void SVGAnimationElement::updateAnimation(float percent, unsigned repeat, SVGSMI
> > -    
> > +
> This change is unrelated, please leave it out.
Done

> > diff --git a/WebCore/svg/animation/SVGSMILElement.h b/WebCore/svg/animation/SVGSMILElement.h
> > @@ -88,7 +88,7 @@ namespace WebCore {
> >          bool isContributing(SMILTime elapsed) const;
> >          bool isInactive() const;
> >          bool isFrozen() const;
> > -        
> > +
> Ditto. Not needed.
Done
> Other than that, looks nice. Please upload a new version.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.