Closed
Bug 610419
Opened 14 years ago
Closed 14 years ago
animation not running in SVG-as-an-image
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla2.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta8+ |
People
(Reporter: dholbert, Assigned: dholbert)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
3.38 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Something from this push... http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=17b72e252d83 seems to have broken animations in SVG-as-an-image, in nightly builds. e.g. load http://blog.dholbert.org/2010/10/svg-as-image.html -- the fish doesn't move in today's nightly. Investigating...
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 1•14 years ago
|
||
Regression is from http://hg.mozilla.org/mozilla-central/rev/04311a70fa33 Probably just need to add an "&& !IsResourceDoc()" check to the added clause there.
Assignee | ||
Comment 2•14 years ago
|
||
Yup, that fixes it. I guess resource documents are "hidden" as far as mIsHidden is concerned -- makes sense. I'd initially thought that any test for animated SVG-as-an-image would have to rely on setTimeout, but I think I can test it without timeouts using mozRequestAnimationFrame/MozBeforePaint/MozAfterPaint. Working up a test right now...
What about animated images in resource documents where the display document is not showing? Are they still stopped with this patch?
Assignee | ||
Comment 4•14 years ago
|
||
Comment on attachment 488955 [details] [diff] [review] fix First, I'll change your question slightly, replacing "display document is not showing" with "display document gets a call to OnPageHide". (I'll address the "display document was never showing in the first place" case aftwerward.) So: Do animated images get stopped in resource documents where the display document gets a call to OnPageHide? For SVG-as-an-image resource documents, the answer is: "Yes, if the display document was the only/last animation consumer for our SVG image." When the display document becomes not showing (via OnPageHide), it calls SetImagesNeedAnimating(PR_FALSE), which calls DecrementAnimationConsumers(), which (if no animation consumers are left for our image) calls SVGDocumentWrapper::StopAnimation, and halts animations in the helper-doc. That all should work correctly since bug 606942. For external resource documents: actually, it looks like we don't want the patch to affect those -- nsDocument::OnPageHide / OnPageShow already forwards PageHide/PageShow calls to its external resource documents -- so they already get unpaused whenever their display document is shown, and re-paused whenever their display document is hidden. So, I think this patch should actually just check "IsBeingUsedAsImage" instead of "IsResourceDoc".[change 1] ======= Now, if the display document was never showing in the first place (unexpected, but was the case in Bug 608295 which regressed this), then: For SVG-as-an-image resource documents: we probably should pause animations in our helper-document with PAUSE_IMAGE. [change 2] For external resource documents: they will behave the same as the display document (never have their animations started in the first place), because their mIsShowing status is kept in sync with the display document. (The original patch here might have broken that, but [change 1] noted above will keep that from breaking). I'll repost the patch with [change 1] & [change 2] noted above, and a reftest.
Attachment #488955 -
Flags: review?(roc)
Assignee | ||
Comment 5•14 years ago
|
||
(In reply to comment #4) > Now, if the display document was never showing in the first place (unexpected, > but was the case in Bug 608295 which regressed this), then: > > For SVG-as-an-image resource documents: we probably should pause animations in > our helper-document with PAUSE_IMAGE. [change 2] ...though (d'oh) of course we have no access to our display document (nor do we necessarily have any single unique display document) for SVG-as-an-image. So -- instead "[change 2]" -- to keep SVG-as-an-image from animating in cases where there's only one display document and that display document never gets shown, I think we might want to start off our helper-document as being paused with PAUSE_IMAGE, and then only unpause it when we get the call to SVGDocumentWrapper::StartAnimation() (e.g. from an animation consumer being added). I'd prefer to make that change in a separate bug, though, since I think it's slightly different in scope and intent from this bug's fix.
Assignee | ||
Comment 6•14 years ago
|
||
Spun off Bug 610491 for Comment 5.
Assignee | ||
Comment 7•14 years ago
|
||
Updated patch with s/IsResourceDoc/mIsBeingUsedAsImage/ (change 1 from comment 4), plus a reftest.
Attachment #488955 -
Attachment is obsolete: true
Attachment #489003 -
Flags: review?(roc)
Assignee | ||
Comment 8•14 years ago
|
||
Comment on attachment 489003 [details] [diff] [review] fix v2 canceling review request while I double-check that the comment about OnPageShow / OnPageHide is correct...
Attachment #489003 -
Flags: review?(roc)
Assignee | ||
Comment 9•14 years ago
|
||
Comment on attachment 489003 [details] [diff] [review] fix v2 (ok, confirmed that the comment is correct)
Attachment #489003 -
Flags: review?(roc)
Assignee | ||
Comment 10•14 years ago
|
||
(This should be blockingb8+ IMHO, since it's a behavior regression w.r.t. beta 7)
Blocks: 608295
Attachment #489003 -
Flags: review?(roc) → review+
blocking2.0: ? → beta8+
Assignee | ||
Comment 11•14 years ago
|
||
Landed: http://hg.mozilla.org/mozilla-central/rev/66de91ff0bbd
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
You need to log in
before you can comment on or make changes to this bug.
Description
•