Ok, you convinced me. I accept your change as a fix for a regression
(I like the 2nd version, because it is a little easier to understand),
but could we also have a Jira item to address the larger issue of
sorting out the duplication between text and sprites?
On a more general note, I wonder if LZNode should just hide
constraints in general from overrides of #construct, rather than the
construct methods having to deal with them? Ideally, they would be
replaced with an initial value, if we could safely invoke the
constraint to compute an initial value, otherwise with, at least null,
to indicate that the attribute was specified (so construct method's
don't try to default them).
On 2008-05-29, at 10:45 EDT, André Bargull wrote:
But this is not just about sprites, the font* attributes will also
be set on LzText directly, it's actually bypassing
"LzNode#__LZapplyArgs(..)"! So you're modifying the args-object
anyway. And to create a copy cannot be done in O(1) and especially
for dhtml, which just need font*, it'd be a waste.
To be honest, there are still many duplicate things in LzText and
LzTextSprite (primarily for swf) (and LzInputtext/
LzInputtextSprite), e.g. setting multiline-ness, which should be
cleaned up. But for fixing this regression-bug, I just wanted to get
constraints working again on font* attributes.
On 5/29/2008 1:41 PM, P T Withington wrote:
Let me see if I understand what is really supposed to happen here:
The sprite needs to see all (or nearly all) the args to the text
construct method. But the sprite is not prepared to deal with
LzInitExprs, so they need to be filtered out. And for the font*
attributes, if they were not specified, they need to be defaulted
by searching the parents. (Aside: this is an old mechanism,
probably should be replaced with CSS?)
Maybe this would be a simple approach:
1) Make a copy of args that will be mutated and passed to the sprite
2) Scan the copy looking for missing font* inits, default them to
the parent values (do these also have to be defaulted in the args
list?)
3) Scan the copy looking for any constraints and remove them, since
the sprite doesn't deal in constraints (do these have to be
replaced with null? or removed altogether?)
An optimization would be to not make the copy of args if there are
no constraints that will need removing. Or, to defer making the
copy until you discover a constraint you need to remove. I think
that is cleaner than removing then replacing.
This is still not perfect. Really, in the case of a constraint,
you probably want to calculate the initial value of the constraint
to pass to the sprite, if the sprite really needs these values to
initialize itself. I don't see how to do that though, since the
constraint is a method on the instance, which at this point is not
fully formed. Perhaps it is only an optimization for the sprite to
get all these values initially, since the sprite has to also
support them changing over time?
On 2008-05-28, at 17:06 EDT, André Bargull wrote:
I wonder if we really should be examining the contract of
__initTextProperties; should it really be getting the whole args
list, or just a subset?
It's just easier to pass the whole args list, let me sum up which
attributes are needed in each runtime:
DHTML:
text+inputtext: font, fontsize, fontstyle
=> easy going
SWF:
text: password, selectable, multiline, font, fontsize, fontstyle,
height
inputtext: password, selectable, multiline, font, fontsize,
fontstyle, text, width, height, maxlength, pattern
=> hmm, a bit more...
So, that's why we're doing this kludge.
And... I see that __initTextProperties seems to already look for
LzInitExpr for some of its args. It seems we ought to be doing
this in one place or the other not some in each.
That depends on the runtime, e.g. dhtml doesn't care about any
constraints for "height", whereas swf needs/wants to do some
special tricks... :-(
Would it be simpler if at the LzText level we just look for the
font properties not being defined and default them to parent
value, and then worry about the LzInitExpr in __initTextProperties?
Then we're going to have three times the same code for each
runtime. Because apparently we need to have these initial font-
infos at the very beginning (for textwidth calculations?).
On 5/28/2008 7:39 PM, P T Withington wrote:
That's a bit clearer.
I wonder if we really should be examining the contract of
__initTextProperties; should it really be getting the whole args
list, or just a subset? Then you wouldn't have to smash args
back and forth. And... I see that __initTextProperties seems to
already look for LzInitExpr for some of its args. It seems we
ought to be doing this in one place or the other not some in each.
Would it be simpler if at the LzText level we just look for the
font properties not being defined and default them to parent
value, and then worry about the LzInitExpr in __initTextProperties?
On 2008-05-28, at 12:27 EDT, André Bargull wrote:
Less voodoo in the LFC? :-)
(To be honest, I wasn't really convinced by my work, too. But on
the other hand, I didn't want to blow up the code resp. slow it
down too much:
e.g. by having two booleans variables per argument, which would
have been a waste as I don't want to save 2^2 states,
or by creating a copy of the args-object through LzInheritedHash.)
So, next try. Maybe that's easier to follow:
var argsMap:Object = {font:fontname, fontsize:fontsize,
fontstyle:fontstyle};
var argsCpy:Object = {};
for (var key:String in argsMap) {
var val:String = argsMap[key];
var hasArg:Boolean = (key in args) ? true :
false;//swf9 forces me to do this...
var initExpr:Boolean = hasArg && args[key] is LzInitExpr;
argsCpy[key] = initExpr ? args[key] :
LzNode._ignoreAttribute;
if (hasArg && !initExpr) {
this[val] = args[key];
} else {
this[val] = args[key] = this.searchParents( val )
[ val ];
}
}
var tsprite:LzTextSprite = (this.sprite cast
LzTextSprite); tsprite.__initTextProperties(args);
for (var key:String in argsCpy) {
args[key] = argsCpy[key];
}
On 5/28/2008 5:18 PM, P T Withington wrote:
Is there any way we could straighten out the logic so it is
easier to understand? I'm having a hard time figuring out
what's going on here. Since you set and read the tri-state
flags within a few lines of each other, it seems there ought to
be an easier way to do this.
On 2008-05-24, at 19:18 EDT, André Bargull wrote:
Update: last change contained a braino, which is now fixed.
Change 20080524-bargull-KxK by [EMAIL PROTECTED] on
2008-05-24 23:55:34
in /home/Admin/src/svn/openlaszlo/trunk
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: make font-attributes constrainable again
New Features:
Bugs Fixed: LPP-6038
Technical Reviewer: ptw
QA Reviewer: promanik
Doc Reviewer: (pending)
Documentation:
Release Notes:
Details:
Font attribute (fontname, fontsize, fontstyle) are special
cased in "LzText#construct(..)". And since constraints are no
longer held in the
$refs object, the special casing need to be adjusted.
Tests:
Files:
M WEB-INF/lps/lfc/views/LzText.lzs
Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080524-bargull-KxK.tar
