[Laszlo-dev] Fwd: reference nave pane has dark background
J Crowley
jcrowley at laszlosystems.com
Fri Dec 5 13:30:23 PST 2008
I've tracked this down and it seems to have broken between 11968 and
11969. r11969 diff is:
Index: WEB-INF/lps/lfc/kernel/swf/LzTextSprite.as
===================================================================
--- WEB-INF/lps/lfc/kernel/swf/LzTextSprite.as (revision 11968)
+++ WEB-INF/lps/lfc/kernel/swf/LzTextSprite.as (revision 11969)
@@ -101,7 +101,9 @@
// if single line, use font line height
// else get height from flash textobject.textHeight
//
- if (args['height'] == null) {
+ // FIXME [2008-11-24 ptw] (LPP-7391) kernel sprites should not be
+ // using LzNode args directly
+ if (! this.owner.hassetheight) {
this.sizeToHeight = true;
// set autoSize to get text measured
textclip.autoSize = true;
@@ -115,7 +117,7 @@
// we got a correct line height from flash.
textclip.autoSize = false;
}
- } else if (! (args.height is LzValueExpr)) {
+ } else if (args['height'] != null) {
textclip._height = args.height;
//this.setHeight(args.height);
}
Index: WEB-INF/lps/lfc/kernel/swf/LzInputTextSprite.as
===================================================================
--- WEB-INF/lps/lfc/kernel/swf/LzInputTextSprite.as (revision 11968)
+++ WEB-INF/lps/lfc/kernel/swf/LzInputTextSprite.as (revision 11969)
@@ -101,7 +101,7 @@
// if empty text content was supplied, use DEFAULT_WIDTH
- //(args.width == null && typeof(args.$refs.width) != "function")
+ // (! this.owner.hassetwidth)
// To compute our height:
@@ -110,7 +110,9 @@
// if single line, use font line height
// else get height from flash textobject.textHeight
//
- if (args['height'] == null) {
+ // FIXME [2008-11-24 ptw] (LPP-7391) kernel sprites should not be
+ // using LzNode args directly
+ if (! this.owner.hassetheight) {
this.sizeToHeight = true;
// set autoSize to get text measured
textclip.autoSize = true;
@@ -124,7 +126,7 @@
// we got a correct line height from flash.
textclip.autoSize = false;
}
- } else if (! (args.height is LzValueExpr)) {
+ } else if (args['height'] != null) {
textclip._height = args.height;
//this.setHeight(args.height);
}
Index: WEB-INF/lps/lfc/kernel/swf9/LzTextSprite.as
===================================================================
--- WEB-INF/lps/lfc/kernel/swf9/LzTextSprite.as (revision 11968)
+++ WEB-INF/lps/lfc/kernel/swf9/LzTextSprite.as (revision 11969)
@@ -182,9 +182,11 @@
// if single line, use font line height
// else get height from flash textobject.textHeight
//
- if (args['height'] == null) {
+ // FIXME [2008-11-24 ptw] (LPP-7391) kernel sprites should
not be
+ // using LzNode args directly
+ if (! this.owner.hassetheight) {
this.sizeToHeight = true;
- } else if (! (args.height is LzValueExpr)) {
+ } else if (args['height'] != null) {
// Does setting height of the text object do the right
thing in swf9?
textclip.height = args.height;
}
Index: WEB-INF/lps/lfc/core/LzNode.lzs
===================================================================
--- WEB-INF/lps/lfc/core/LzNode.lzs (revision 11968)
+++ WEB-INF/lps/lfc/core/LzNode.lzs (revision 11969)
@@ -149,7 +149,22 @@
var _instanceChildren:Array = null;
+ /** @access private */
+ var __LzValueExprs = null;
/**
+ * Private interface to a very limited set of LFC classes (LzView,
+ * LzReplicationManager) that need to know in `construct` when an
+ * attribute will be constrained later by `__LZapplyArgs`.
+ *
+ * @param String attr: The name of the attribute to look up
+ * @returns boolean: True if it will be constrained (barring the
+ * constructor itself overriding it!)
+ *
+ * @access private */
+ function __LZhasConstraint(attr:String) {
+ return attr in this.__LzValueExprs;
+ }
+ /**
* @param LzNode parent: a node above this one in the hierarchy -- not
* necessarily the immediate parent -- that will decide where this
node goes
* @param Object attrs: a dictionary of attributes used to initialize
@@ -275,12 +290,35 @@
delete iargs["$datapath"];
}
+ /// See LzNode/construct documentation for the purpose of this
+ // filtering/copying
+
+ // Remove the init expressions from the args when calling construct
+ var cargs = this.__LzValueExprs = {};
+ for (var key in iargs) {
+ var val = iargs[key];
+ if (val is LzValueExpr) {
+ cargs[key] = val;
+ delete iargs[key];
+ }
+ }
this.construct( parent , iargs );
-
// Construct may, through many tangled webs of replication and
// placement, actually end up deleting us! Bail out.
if (this.__LZdeleted) { return; }
+ // Now put them back, unless they have been overridden
+ for (var key in cargs) {
+ // TODO [2008-12-02 ptw] Some legacy constructors appear to
+ // depend on overriding a constraint. IWBNI the only thing a
+ // constructor could do would be to remove a constant arg that
+ // it had handled.
+ if (! (key in iargs)) {
+ iargs[key] = cargs[key];
+ }
+ }
+ this.__LzValueExprs = null;
+
// this.setClassEvents( this.constructor );
this.__LZapplyArgs( iargs , true );
@@ -671,7 +709,7 @@
/**
* The <code>construct</code> method of a node is called as part of
* the process of constructing a node. It is called <i>after</i>
- * attributes with constant initial values have been filled in, but
+ * attributes with constant initial values of the class have been
filled in, but
* before any attributes that have setters or have been constrained
* have been applied.
*
@@ -717,12 +755,25 @@
* @param LzNode parent: The node that encloses this node in source,
* or the node to which to attach this node.
* @param Object args: A dictionary of attribute initializations for
- * attributes that have setters or that have been constrained. In
- * general, your constructor should not modify this dictionary. It
- * may verify that an attribute is named in the dictionary to
- * determine if the attribute will be modified by a setter or
- * constraint.
+ * attributes that have setters or that have been supplied by the
+ * instance declaration. In
+ * general, your construct method should <em>not</em> modify this
dictionary. It
+ * may delete a value from the dictionary to indicate that it has
+ * been handled by the construct method (and should not be further
processed).
*
+ * @devnote NOTE [2008-11-24 ptw] The construct method <em>only</em>
+ * sees constant initial arguments. Non-constant bindings
+ * (constraints) are processed by <code>LzNode</code> to get
+ * inheritance/overriding correct, but they are an internal
+ * protocol, not exposed to user construct methods. In general,
+ * overrides of construct should <em>only</em> be looking at
+ * <code>args</code> as a possible optimization (e.g., it may be
+ * possible to optimize some aspect of construction if certain
+ * attributes have constant initial values); or, if an attribute can
+ * <em>only</em> have a constant initial value, which can be
+ * enforced by processing that value in the constructor, removing it
+ * from <code>args</code>, and having a setter that does nothing (or
+ * signals an error in debug mode).
*/
function construct ( parent , args ){
// Set applyArgs ordering kludges. LzView overrides these
@@ -2109,12 +2160,12 @@
* debugger.
*/
function getDebugIdentification (){
- var s = this['constructor'].tagname + " ";
- if ( this.name != null ){
- s += " name: " + this.name + " ";
+ var s = this['constructor'].tagname;
+ if ( this['name'] != null ){
+ s += " name: " + this.name;
}
- if ( this.id != null ){
- s += " id: " + this.id + " ";
+ if ( this['id'] != null ){
+ s += " id: " + this.id;
}
return s;
}
Index: WEB-INF/lps/lfc/views/LzInputText.lzs
===================================================================
--- WEB-INF/lps/lfc/views/LzInputText.lzs (revision 11968)
+++ WEB-INF/lps/lfc/views/LzInputText.lzs (revision 11969)
@@ -103,7 +103,7 @@
* @access private
*/
override function construct ( parent , args ){
- this.password = ('password' in args && (! (args.password is
LzValueExpr))) ? (!! args.password) : false;
+ this.password = ('password' in args) ? (!! args.password) : false;
this.focusable = true;
super.construct.apply(this, arguments);
this._onfocusDel = new LzDelegate( this , "_gotFocusEvent" , this,
Index: WEB-INF/lps/lfc/views/LzText.lzs
===================================================================
--- WEB-INF/lps/lfc/views/LzText.lzs (revision 11968)
+++ WEB-INF/lps/lfc/views/LzText.lzs (revision 11969)
@@ -431,8 +431,11 @@
this.$lzc$set_height(h);
}
}
-}
+};
+static var fontArgToAttr:Object = {font: 'fontname', fontsize:
'fontsize', fontstyle: 'fontstyle'};
+
+
/**
* @access private
*/
@@ -445,45 +448,41 @@
super.construct.apply(this, arguments);
this.sizeToHeight = false;
-
- var fontMap:Object = {font:'fontname', fontsize:'fontsize',
fontstyle:'fontstyle'};
- var fontCpy:Object = {};
- for (var key:String in fontMap) {
- var hasArg:Boolean = (key in args) ? true : false;//can't just
write "key in args" in swf9..
- var initExpr:Boolean = hasArg && args[key] is LzInitExpr;
- fontCpy[key] = initExpr ? args[key] : LzNode._ignoreAttribute;
- if (hasArg && !initExpr) {
- // a normal value, just install it
- this[fontMap[key]] = args[key];
- } else {
- // font-attribute wasn't specified or it was a constraint,
- // need to lookup for initial font-attribute
- var val:String = fontMap[key];
- this[val] = args[key] = this.searchParents( val )[ val ];
- }
+
+ // Install constant/default font attributes
+ for (var arg:String in LzText.fontArgToAttr) {
+ var attr:String = LzText.fontArgToAttr[arg];
+ // Default missing args
+ if (! (arg in args)) {
+ args[arg] = this.searchParents(attr)[attr];
+ }
+ this[attr] = args[arg];
}
-
- var tsprite:LzTextSprite = (this.sprite cast LzTextSprite);
+
+ var tsprite:LzTextSprite = (this.sprite cast LzTextSprite);
+ // FIXME [2008-11-24 ptw] (LPP-7391) We should not be passing node
+ // init args across the kernel API, there should be a more
+ // explicit API that isolates the kernel from the node
+ // implementation details
tsprite.__initTextProperties(args);
-
- for (var key:String in fontCpy) {
- // if font-attribute was a constraint, copy it back,
- // else set it to LzNode._ignoreAttribute,
- // so it won't be processed by LzNode#__LZapplyArgs(..)
- args[key] = fontCpy[key];
+
+ // Remove constant/default font attributes, so they won't be
+ // re-handled by LzNode/__LZapplyArgs
+ for (var arg:String in LzText.fontArgToAttr) {
+ delete args[arg];
}
this.yscroll = 0;
this.xscroll = 0;
- this.resize = ('resize' in args && (! (args.resize is
LzValueExpr))) ? (!! args.resize) : this.resize;
+ this.resize = ('resize' in args) ? (!! args.resize) : this.resize;
this.$lzc$set_resize(this.resize);
- if ('maxlength' in args && (! (args.maxlength is LzValueExpr)) &&
args.maxlength != null) {
+ if (args['maxlength'] != null) {
this.$lzc$set_maxlength(args.maxlength);
}
- this.text = (!('text' in args) || args.text == null || args.text
instanceof LzInitExpr) ? "" : args.text;
+ this.text = (args['text'] != null) ? args.text : "";
if(this.maxlength != null && this.text.length > this.maxlength){
this.text = this.text.substring(0, this.maxlength);
}
@@ -498,7 +497,7 @@
// + if text is single line:
// if no width was supplied and there's no constraint, measure
the text width:
// if empty text content was supplied, use DEFAULT_WIDTH
- if (args.width == null) {
+ if (! this.hassetwidth) {
if (this.multiline) {
args.width = this.parent.width;
} else {
@@ -521,15 +520,15 @@
// if single line, use font line height
// else get height from flash textobject.textHeight
//
- if (!this.hassetheight) {
+ if (! this.hassetheight) {
this.sizeToHeight = true;
- } else if (! (args.height is LzValueExpr)) {
+ } else if (args['height'] != null) {
this.$lzc$set_height(args.height);
}
// Default the scrollheight to the visible height.
this.scrollheight = this.height;
- if ('pattern' in args && (! (args.pattern is LzValueExpr)) &&
args.pattern != null) {
+ if (args['pattern'] != null) {
this.$lzc$set_pattern(args.pattern);
}
Index: WEB-INF/lps/lfc/views/LaszloView.lzs
===================================================================
--- WEB-INF/lps/lfc/views/LaszloView.lzs (revision 11968)
+++ WEB-INF/lps/lfc/views/LaszloView.lzs (revision 11969)
@@ -438,25 +438,25 @@
this.__makeSprite(args);
- if ( 'width' in args ){
+ if ( 'width' in args || this.__LZhasConstraint('width')) {
this.hassetwidth = true;
this.__LZcheckwidth = false;
}
- if ( 'height' in args ){
+ if ( 'height' in args || this.__LZhasConstraint('height')) {
this.hassetheight = true;
this.__LZcheckheight = false;
}
- if ('clip' in args && args['clip'] && (! (args.clip instanceof
LzInitExpr)) ){
+ if (args['clip'] != null){
this.clip = args.clip;
this.makeMasked();
}
- if ('stretches' in args && args['stretches'] != null && (!
(args.stretches instanceof LzInitExpr)) ){
+ if (args['stretches'] != null){
this.$lzc$set_stretches(args.stretches);
args.stretches = LzNode._ignoreAttribute;
}
- if (args['resource'] != null && (! (args.resource instanceof
LzInitExpr)) ){
+ if (args['resource'] != null){
this.$lzc$set_resource( args.resource );
args.resource = LzNode._ignoreAttribute;
}
Index: WEB-INF/lps/lfc/data/LzReplicationManager.lzs
===================================================================
--- WEB-INF/lps/lfc/data/LzReplicationManager.lzs (revision 11968)
+++ WEB-INF/lps/lfc/data/LzReplicationManager.lzs (revision 11969)
@@ -290,6 +290,12 @@
} else {
if (!view.isinited) {
var via:Object = view._instanceAttrs;
+ // NOTE [2008-12-01 ptw] I did not use
+ // LzNode/__LZhasConstraint here, because the original code
+ // only looks for visible in the instance attributes. I
+ // can't tell if that was an error, or the designer
+ // intentionally did not want to consider the setting of
+ // visible in the class
if (via != null && 'visible' in via && !(via.visible is
LzInitExpr)) {
this.visible = via.visible;
} else {
@@ -306,6 +312,11 @@
var ia:Object = view._instanceAttrs;
var oa:Object = odp._instanceAttrs;
+ // NOTE [2008-12-01 ptw] I did not use LzNode/__LZhasConstraint
+ // here, because the original code only looks for visible in the
+ // instance attributes. I can't tell if that was an error, or the
+ // designer intentionally did not want to consider the setting of
+ // visible in the class
if (ia && 'datapath' in ia && ia.datapath is LzAlwaysExpr) {
// <view datapath="${ ... }"/>
// we need to mask this constraint
Index: demos/lzpixmobile/lib/hintedlayout.lzx
===================================================================
--- demos/lzpixmobile/lib/hintedlayout.lzx (revision 11968)
+++ demos/lzpixmobile/lib/hintedlayout.lzx (revision 11969)
@@ -20,10 +20,10 @@
<attribute name="yinset" value="0"/>
<!--- A pixel amount to use between the views controlled by the
layout in
the x axis. -->
- <attribute name="xspacing" value="null" />
+ <attribute name="xspacing" value="${this.spacing}" />
<!--- A pixel amount to use between the views controlled by the
layout in
the y axis. -->
- <attribute name="yspacing" value="null" />
+ <attribute name="yspacing" value="${this.spacing}" />
<!--- If given, a number of miliseconds to use to animate the
views in to
place.-->
<attribute name="duration" value="0"/>
@@ -36,11 +36,6 @@
<!--- @keywords private -->
<method name="construct" args="parent, args">
super.construct(parent, args);
- <!-- The class default value is this.spacing, but the user
may override that in args -->
- <!-- If x and y spacing are not specified they take on the
default value -->
- var defaultSpacing = ('spacing' in
args)?args.spacing:this.spacing;
- if (! ('xspacing' in args)) args.xspacing = defaultSpacing;
- if (! ('yspacing' in args)) args.yspacing = defaultSpacing;
this.doupdateDelegate = new lz.Delegate(this, "doupdate");
</method>
P T Withington wrote:
> Just to try to clarify my reasoning:
>
> In my experience, it is best practice to remove the work-around when
> the underlying bug is fixed, otherwise the underlying bug may recur
> and you will not notice it. The wallpaper metaphor is a standard
> metaphor, I'm just passing it on. It holds more meaning for me than
> 'canary in a coal mine' metaphors, but maybe that works for others.
> Removing the work-around is like replacing a dead canary.
>
> On 2008-12-05, at 09:47EST, P T Withington wrote:
>
>> I filed a bug. http://jira.openlaszlo.org/jira/browse/LPP-7439
>>
>> There is nothing wrong with setting the bgcolor explicitly.
>>
>> What is wrong is that a previously working program changed its
>> behavior inexplicably and by papering over the bug, we risk missing a
>> more systemic issue. I'm sorry if I insulted you by suggesting you
>> like floral-design wallpaper, it is just the standard software
>> engineering metaphor, not intended to impugn your character. I would
>> have said nav.xml is "the canary in the coal mine", but then the
>> software geeks would not know what I was talking about.
>>
>>
>> On 2008-12-05, at 09:41EST, Lou Iorio wrote:
>>
>>> I think I'm insulted.
>>>
>>> What's wrong with setting the background color explicitly instead of
>>> relying on the default?
>>>
>>> I was going to file another bug for the color issue, but I don't
>>> know what
>>> category under which to file it. Perhaps you could file it.
>>>
>>> BTW, I would never consider wallpaper.
>>>
>>> On Dec 5, 2008, at 10:32 AM, P T Withington wrote:
>>>
>>>> This is called 'papering over' the bug -- equivalent to when you
>>>> notice a crack in the foundation of your house has echoed up into
>>>> the second-story bedroom wall and you 'fix it' by installing new
>>>> wallpaper.
>>>>
>>>> On 2008-12-04, at 18:04EST, Lou Iorio wrote:
>>>>
>>>>> I can make the change (if Don doesn't mind), but I see no reason
>>>>> why it should
>>>>> be temporary. However, you have so much more knowledge than I, why
>>>>> don't
>>>>> _you_ file the bug to Josh? :-)
>>>>>
>>>>> On Dec 4, 2008, at 6:57 PM, P T Withington wrote:
>>>>>
>>>>>> So you could make the temp fix, mark it to be reverted when the
>>>>>> bug you file to Josh is fixed. :)
>>>>>>
>>>>>> On Dec 4, 2008, at 17:29, Lou Iorio <lou at louiorio.com> wrote:
>>>>>>
>>>>>>> I agree that we need to know the root cause in case this breaks
>>>>>>> someone
>>>>>>> else's code, but this seems like a good solution for the reference.
>>>>>>>
>>>>>>>
>>>>>>> On Dec 4, 2008, at 6:08 PM, P T Withington wrote:
>>>>>>>
>>>>>>>> We need to know the root cause... Perhaps assign to Josh, who
>>>>>>>> has played with colors a lot lately?
>>>>>>>>
>>>>>>>> On 2008-12-04, at 17:02EST, Donald Anderson wrote:
>>>>>>>>
>>>>>>>>> FWIW, changing the three tabpanes in docs/src/nav/nav.lzx to
>>>>>>>>> have an explicit bgcolor fixes this:
>>>>>>>>>
>>>>>>>>> < <tabpane width="${parent.width}">
>>>>>>>>> ---
>>>>>>>>> > <tabpane width="${parent.width}" bgcolor="0xffffff">
>>>>>>>>>
>>>>>>>>> Should I do that, or does someone want to chase the root cause?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Dec 4, 2008, at 11:05 AM, Lou Iorio wrote:
>>>>>>>>>
>>>>>>>>>> Anyone know anything more about this?
>>>>>>>>>>
>>>>>>>>>> Begin forwarded message:
>>>>>>>>>>
>>>>>>>>>>> From: Donald Anderson <dda at ddanderson.com>
>>>>>>>>>>> Date: December 4, 2008 11:58:07 AM GMT-04:00
>>>>>>>>>>> To: Lou Iorio <lou at louiorio.com>
>>>>>>>>>>> Cc: Amy Muntz <amuntz at laszlosystems.com>
>>>>>>>>>>> Subject: Re: reference nave pane has dark background
>>>>>>>>>>>
>>>>>>>>>>> Hi Lou,
>>>>>>>>>>>
>>>>>>>>>>> No it wasn't intentional and I don't know what caused it. I
>>>>>>>>>>> don't remember
>>>>>>>>>>> doing any doc work for about the last month....
>>>>>>>>>>>
>>>>>>>>>>> - Don
>>>>>>>>>>>
>>>>>>>>>>> On Dec 4, 2008, at 8:56 AM, Lou Iorio wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Don,
>>>>>>>>>>>>
>>>>>>>>>>>> I don't know if this is a recent change (I think it is),
>>>>>>>>>>>> but the nav pane background
>>>>>>>>>>>> is considerably darker than in previous versions. It now
>>>>>>>>>>>> displays black text on
>>>>>>>>>>>> a dark gray background. To me, at least, this is harder to
>>>>>>>>>>>> read. Was this change
>>>>>>>>>>>> intentional?
>>>>>>>>>>>>
>>>>>>>>>>>> This screen shot shows the current reference (left) and the
>>>>>>>>>>>> 4.1.1 reference (left).
>>>>>>>>>>>>
>>>>>>>>>>>> <navpane.tiff>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>>
>>>>>>>>>>> Don Anderson
>>>>>>>>>>> Java/C/C++, Berkeley DB, systems consultant
>>>>>>>>>>>
>>>>>>>>>>> voice: 617-547-7881
>>>>>>>>>>> email: dda at ddanderson.com
>>>>>>>>>>> www: http://www.ddanderson.com
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>>
>>>>>>>>> Don Anderson
>>>>>>>>> Java/C/C++, Berkeley DB, systems consultant
>>>>>>>>>
>>>>>>>>> voice: 617-547-7881
>>>>>>>>> email: dda at ddanderson.com
>>>>>>>>> www: http://www.ddanderson.com
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>>
>
>
More information about the Laszlo-dev
mailing list