Closed Bug 571863 Opened 14 years ago Closed 14 years ago

Animation of "width" on inner-<svg> does not cause a repaint

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- betaN+
status2.0 --- wanted

People

(Reporter: jwatt, Assigned: longsonr)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files, 2 obsolete files)

Animation of "width" on <svg> does not cause a repaint.
Attached image testcase
Oops, that should be setTimeAndSnapshot(1, true), not setTimeAndSnapshot(5, true).
Should/does attributeType="XML" make any difference?
Not on inner-<svg>, since the CSS property 'width' isn't relevant there. The underlying change does happen, but this is just an invalidation issue. If you resize the window then it updates correctly.

In fact even on outer-<svg> attributeType wouldn't be necessary, since the CSS property 'width' defaults to 'auto' effectively allowing the XML attribute to be "used".
Blocks: enablesmil
Does setting the width attribute from script invalidate correctly?  If so, where does that happen?
As it happens, no, setting the 'width' attribute on inner-<svg> doesn't work either. That's really a separate bug and a regression from bug 545550 though, so I've filed bug 571969 for that.

Regarding this bug, probably the issue is that this comment is now wrong:

http://hg.mozilla.org/mozilla-central/annotate/7e685b61a291/layout/svg/base/src/nsSVGInnerSVGFrame.h#l59

since nsSVGElement::DidAnimateLength triggers invalidation by calling AttributeChanged on the element's frame.

The strange thing is that when that comment was added (see bug 301628 comment 16) nsSVGSVGElement::DidModifySVGObservable didn't do anything explicit with width/height. I guess it did fall through blindly and call NotifyViewportChange though, but that didn't take account of the viewport established by the inner-<svg>. We should do that when we add nsSVGInnerSVGFrame::AttributeChanged to fix this bug.

If someone doesn't get to writing a patch before me, I'll make one after I've fixed the length list bug.
status2.0: --- → wanted
Summary: Animation of "width" on <svg> does not cause a repaint → Animation of "width" on inner-<svg> does not cause a repaint
Attached patch patch (obsolete) — Splinter Review
Attachment #451202 - Flags: review?(jwatt)
You've been a long time with this Jonathan.
Attached patch updated patch with x, y fix (obsolete) — Splinter Review
Attachment #451202 - Attachment is obsolete: true
Attachment #481158 - Flags: review?(dholbert)
Attachment #451202 - Flags: review?(jwatt)
Comment on attachment 481158 [details] [diff] [review]
updated patch with x, y fix

Looks good!
Attachment #481158 - Flags: review?(dholbert) → review+
(Needs reftest(s) before landing, though.)
Blocks: 601934
low risk regression from firefox 3.6 with reftest
Assignee: nobody → longsonr
Attachment #481158 - Attachment is obsolete: true
Attachment #481326 - Flags: approval2.0?
Probably worthy of blocker status (rather than needing approval), since this is a rendering regression w.r.t. Gecko 1.9.2 / Firefox 3.6.
Status: NEW → ASSIGNED
blocking2.0: --- → ?
Keywords: regression, testcase
Attachment #481326 - Flags: approval2.0?
Whiteboard: [needs landing]
Attachment #481326 - Attachment is patch: true
Landed:
 http://hg.mozilla.org/mozilla-central/rev/1529931ada9e
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.