On 2010-11-15, at 18:33, André Bargull wrote:
>> style: Bug discovered by new code -- the color type auto-quotes
>> its value property. If you want the value to be an expression,
>> you must use ${}.
>
> The "menuitembgcolor" uses the old `when="once|always|immediately"` syntax to
> declare a constraint, basically `<attribute name="menuitembgcolor"
> value="textfieldcolor" when="once" type="color"/>` should act the same as
> `<attribute name="menuitembgcolor" value="$once{textfieldcolor}"
> type="color"/>`.
Wow. Apparently this is due a a confluence of changes/bugs.
1) I can't figure out when this changed (if ever), but in LzNodeModel#1656 we
decide that if a value is in ${}'s, we do nothing at compile time and pass it
straight through to be set into the attribute (according to the when time).
There is a potential bug here, because that means the old `when` property is
_not_ treated the same as ${}. To fix this, we should follow this path for any
when setting other than `immediate`.
2) In r11640, Max took me at my word and made the following change:
throw new CompilationError(source, name, e);
}
}
- // TODO: [2003-05-02 ptw] Wrap non-constant colors in
- // runtime parser
+ value = "LzColorUtils.convertColor('" + value + "')";
} else if (type == ViewSchema.CSS_TYPE) {
if (when.equals(WHEN_IMMEDIATELY)) {
try {
Thus values that the compiler can't recognize as a color are passed as a string
to the runtime parser. Apparently in the past, the parser never warned for
strings that it could not parse (although it does explicitly check for the
string `'null'`).
3) The change reviewed here _does_ give a warning for things it can't parse,
and the string `'textfieldcolor'` is one of those.
So, it would seem to me that menuitembgcolor has not been defaulting to
textfieldcolor for some time now...
---
Proposed solution(s):
1) I think it is correct that an immediate value for an attribute of type color
should be a valid CSS color specification, and hence should be quoted. (There
is a tiny bug here that the compiler should use ScriptCompiler.quote, not
literal `'`s.)
2) The legacy color parser will try to coerce any string as a number before
giving up. This is why you can still specify a color as a number, even though
that is not technically a valid CSS color spec. We ought to issue a warning
for that, both at compile- and run-time, rather than letting this lossage
continue to creep in.
3) The compile should be fixed to make the legacy `when` values of 'once' and
'always' follow the same path as `${}`. This has been broken for some time, so
there is a chance that by making it work correctly some legacy code will be
broken. I think that is a risk we have to accept. The alternative would be to
simply remove support for the `when` property (since it is presently
non-functional).
