W3C home > Mailing lists > Public > public-qa-dev@w3.org > July 2008

Re: Leftover code in the CSS Validator ?

From: Yves Lafon <ylafon@w3.org>
Date: Fri, 25 Jul 2008 10:05:18 -0400 (EDT)
To: Damien B <night.kame@gmail.com>
Cc: public-qa-dev@w3.org
Message-ID: <Pine.LNX.4.64.0807250957070.24569@ubzre.j3.bet>

On Thu, 24 Jul 2008, Damien B wrote:

>
> Hello,
>
> while doing some experiments to write a GUI for the CSS Validator, I've found 
> a strange behaviour.
> When validating http://www.w3.org, I've got some errors on:
>
> voice-family: inherit;
>
> Which seems perfectly acceptable.
> The relevant code in ACssVoiceFamily is here:
> if (val.equals(inherit)) {
>    if(value.getCount() > 1) {
> 	throw new InvalidParamException("unrecognize", ac);
>    }
>    inheritValue = true;
>    return;
> }
>
> Which is fine and executed correctly, except that "val" is obtained by
> CssValue val = value.getValue();
> and getValue() does not consume the token.
>
> Back in CssParser:
> CssProperty p = handleDeclaration(string_property.toLowerCase(),
>                                                  values, important);
> // Did the property recognize all values in the expression ?
> if (!values.end() && ac.getMedium() == null) {
>     addError(new InvalidParamException("unrecognize", "", ac),
>                             values);
> } else {
>   // ok, return the new property

>   {if (true) return p;}  <--- smells like temporary code :-D

Ouch, that bad. You will probably find other horrors like that in other 
parts of the code...

Now the issue is... is the other code snippet you mentionned doing the 
right thing by not eating the value, or is this part faulty.

In any case, some cleanup is needed. Profiles like ACss did lack some 
caring. Has this bug been reported in Bugzilla ? (that's my primary source 
of infomation when one need to squash a bug, or more).

> So, in that case, "!values.end()" is true, and in my test, ac.getMedium() is 
> null, thus the reported errors.
>
> But, with the CSS Validator online service, there is no way to set Medium to 
> null, thus the "errors" go unnoticed.
> So basically, in the online validator, the execution path is this one:
> if (false) {
> } else {
>   {if (true) return p;}
> }
>
> So... I don't know how to conclude. What happens next? :-)
>
> Damien B
>

-- 
Baroula que barouleras, au tiéu toujou t'entourneras.

         ~~Yves
Received on Friday, 25 July 2008 14:05:53 GMT

This archive was generated by hypermail 2.2.0+W3C-0.50 : Thursday, 19 August 2010 18:12:49 GMT