Closed Bug 536660 Opened 10 years ago Closed 2 months ago

Keep around anonymous nodes for <use>, even when the <use> doesn't have any frames

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: dholbert, Unassigned)

References

Details

Attachments

(1 file)

Currently, the anonymous content for <use> elements gets removed from the document as soon as the <use> element's frames go away.  See in particular nsSVGUseFrame::Destroy(), which calls nsSVGUseElement::DestroyAnonymousContent().

This caused problems with SMIL animation in the testcases posted on Bug 534975, because we expect that the content tree's structure won't change during the course of a sample. In particular, we expect that nodes in the document at the beginning of the sample should still be in the document at the end of the sample.

But the problem was if we apply a 'display: none' style the <use> element during an animation sample, then its frame gets destroyed, and right now that will remove all of its anonymous content from the document.

We've worked around this in that bug, but Roc suggests a better solution in Bug 534975 comment #17:
> have the <use> anonymous nodes owned by the element and present whether the
> <use> element has a frame or not.
(In reply to comment #0)
> But the problem was if we apply a 'display: none' style the <use> element
> during an animation sample, then its frame gets destroyed, and right now that
> will remove all of its anonymous content from the document.

(...which is a problem if some of that removed anonymous content was the target of animations and was queued up for compositing later in the same animation sample.)
See Also: → 534975
Attached image Test case
With the attached test case we end up generating animation functions ad infinitum.
Blocks: 562815
In bug 562815 (Don't always force-recompose animations of CSS properties) we ran into trouble because of this bug.

I don't properly understand it but it seems to be something like this:

* In the test case (attachment 550939 [details]) the <set> is anonymous content of <use> and we regenerate the anonymous content of the <use> every sample (because it sets display to none)
* In nsSMILCSSProperty, when aPreventCachingOfSandwich is PR_TRUE for display:none (as it is now) we somehow clear the anonymous content because we were always set the property
* But if we allow it to be sometimes PR_FALSE (which is what bug 562815 tries to fix) we don't end up re-setting that property and don't end up clearing the anonymous content which gets generated every sample

That may not be accurate (at all) but I think it's something of that ilk.

If we allow aPreventCachingOfSandwich to be PR_FALSE with display:none /content/smil/crashtests/572938-1.svg gets into an infinite update situation and, on Windows at least, the UI becomes unresponsive (you can still click buttons but nothing happens).

For bug 56281 we worked around this by just forcing aPreventCachingOfSandwich to be PR_TRUE for display:

http://mxr.mozilla.org/mozilla-central/source/content/smil/nsSMILCSSProperty.cpp#172

The downside is that it means we miss out on optimising animation of the display property which is probably one of the most commonly animated properties (particularly with <set>).

In fixing this bug, we should check if the hack to nsSMILCSSProperty mentioned above can be removed.

<use> elements are now based on shadow trees, nsSVGUseFrame no longer has a destroy method.

Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.