525261 - nsSVGUtils::ReportToConsole if SMIL target not a property and attributeType!="XML"

Status: RESOLVED WONTFIX

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

In the case that a SMIL animation has attributeName set to a value that is not recognized as a CSS property and it does not have attributeType explicitly set to "XML", I think we should warn in the console using nsSVGUtils::ReportToConsole to raise awareness that they should explicitly use attributeType="XML" as their content may change behavior in future if a new clashing property is added.

Of course nsSVGUtils::ReportToConsole first needs fixed up to only report once per occurrence.

Comment 1

[Robert Longson [:longsonr]]

Folks could easily write something like

animationElement.setAttribute("attributeName", "whatever");
animationElement.setAttribute("attributeType", "XML");

Won't we end up with warnings after the first line has executed?

Comment 2

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to comment #1)
> Folks could easily write something like
> 
> animationElement.setAttribute("attributeName", "whatever");
> animationElement.setAttribute("attributeType", "XML");
> 
> Won't we end up with warnings after the first line has executed?

That's a good point. Perhaps we could report these errors at sample time? We already need to do this for various animation attributes, because we don't parse them until sample time. See:

http://mxr.mozilla.org/mozilla-central/source/content/smil/nsSMILAnimationFunction.cpp#134

Of course in this case, we'd be adding that kind of logic to the compositor instead of the animation function. Perhaps here:

http://mxr.mozilla.org/mozilla-central/source/content/smil/nsSMILCompositor.cpp#90

But perhaps something of that logic would work there as well? i.e. somehow flag when the attribute is changed, then report the error during the sample and reset the flag?

I think that's better than generating the error string on every call and throwing it away every time except one.

A series of 'setAttribute' calls generally does not trigger a sample. (But some other script calls do--e.g. beginElement or a setAttribute followed by querying an animated value.)

Comment 3

[Jonathan Watt [:jwatt]]

I was thinking that nsSVGUtils::ReportToConsole should maintain some sort of hash based off the pointer of the object that has the error and the error message itself. It would be called later as Brian suggests, and if the error has already been reported, it wouldn't report it a second time.

Comment 4

[Robert Longson [:longsonr]]

How would you clean up the hash when the page was reloaded/tab closed etc?

Comment 5

[Jonathan Watt [:jwatt]]

Hang it off the document object? Whatever works. I haven't got that far yet. :-)

Comment 6

[Robert Longson [:longsonr]]

attributeType is obsolete per bug 1062106