Re: udom-svg-224 review

On Mon, 13 Oct 2008 16:44:11 +0200, Erik Dahlström <ed@opera.com> wrote:

>
> Hello svg-wg,
>
> I've had a look at udom-svg-224[1] test, and I think there are a few problems with it.
>
>>From TraitAccess[2]:
> <spec>
> Note about invalid/unsupported trait values: There are two situations where the various trait setter methods (such as setTrait, setFloatTrait or setPathTrait methods) consider a value invalid and throw a DOMException with the INVALID_ACCESS_ERR code. The first situation is when the trait value is invalid with regards to its definition (for example, trying to set the 'stroke-linejoin' trait to 'foo' would cause this exception). The trait methods will consider the value to be invalid if it is in error.
> </spec>
>
> The test tries to set 'begin' to 'invalid' (which is possibly an unsupported value) and expects INVALID_ACCESS_ERR to be thrown. However, the definition of the 'begin' attribute[3] doesn't specify that invalid/unsupported values are 'in error'. Either the spec needs to be updated if the intent is to throw for values that are 'unsupported values' or the test needs to be changed to remove these subtests. The example for 'stroke-linejoin' seems to imply that 'unsupported values' was what was intended, not 'in error'. Do we even have any attributes/properties that declare themself 'in error' nowadays? Also note that the old CR version[4], used to throw the exception NOT_SUPPORTED_ERR for the 'stroke-linejoin' example.
>
> Also the begin value is actually a valid event-value for implementations that support the 'invalid' event, which happens to be the case for Opera since it's part of the webforms spec[5]. So it would be better to make the begin value syntactically invalid in some other way, for example by setting the value 'invalid+'.
>
> Proposed course of action:
>
> - Changes in test:
>   s/invalid/invalid+/
>
>   Allow implementations that don't handle the audio or video formats to pass
>
>   Fix the error messages for the set*Trait calls near the end (mostly replace get* with set*)
>
>   (Yes I have these fixes in a buffer ready to commit)
>
> - Changes in spec:
>   I think the intent is to throw INVALID_ACCESS_ERR for invalid traits that are handled by a UA, and NOT_SUPPORTED_ERR for traits that the UA doesn't handle.
>   If so then the change would simply be to replace 'in error' with 'an unsupported value'. Or possibly to include both of them.
>
> Cheers
> /Erik
>
> [1] http://dev.w3.org/SVG/profiles/1.2T/test/svg/udom-svg-224-t.svg
> [2] http://dev.w3.org/SVG/profiles/1.2T/publish/svgudom.html#svg__TraitAccess
> [3] http://dev.w3.org/SVG/profiles/1.2T/publish/animate.html#BeginAttribute
> [4] http://www.w3.org/TR/2006/CR-SVGMobile12-20060810/svgudom.html#svg__TraitAccess
> [5] http://www.whatwg.org/specs/web-forms/current-work/#forms-events-model

Forgot to mention that it fixes one of the "at risk" testcases. A quick discussion might solve the remaining spec issue too. Let's discuss this today if we have time.

Also there are similar issues with the udom-svg-220 testcase.
/Erik

-- 
Erik Dahlstrom, Core Technology Developer, Opera Software
Co-Chair, W3C SVG Working Group
Personal blog: http://my.opera.com/macdev_ed

Received on Tuesday, 14 October 2008 07:51:42 UTC