|
|
|
[
Permlink
| « Hide
]
Amy Muntz - 11/Sep/07 09:16 AM
Cloned and reopened to fix the source of the problem. Downgraded to INFO as a workaround in wafflecone.
This seems like it may be the result of (and, thus, duplicate of)
The "p is null" thing is an intentional Debugger output ( The specific cause, in this case, is an oninit call to _updateSelectionByIndex(0,false,true); which in turn has the line var nodes = dp.xpathQuery(this.itemdatapath); If I comment out this oninit call, and then call it later manually by invoking it in the debugger, the warning doesn't appear. I could develop a workaround for this in basedatacombobox.lzx, but I think this is a deeper problem with data loading that needs to be fixed. Okay, I've been playing around with this one and I think I've figured out a workaround. I'll send it out for review later today, but I'm going to test it a little more to make sure. It looks like this also was a problem with other calls and things to the same method. I think I've managed to stamp them all out. This was also breaking selectfirst for remote datasets, it seems like.
Fix revised by Andre, re-reviewed and approved. Checkin transcript follows:
Author: bargull Date: 2007-11-07 14:01:54 -0800 (Wed, 07 Nov 2007) New Revision: 7184 Modified: openlaszlo/trunk/lps/components/base/basedatacombobox.lzx Log: Change 20071106-bargull-8 by bargull@dell--p4--2-53 on 2007-11-06 21:06:22 in /home/Admin/src/svn/openlaszlo/trunk for http://svn.openlaszlo.org/openlaszlo/trunk Summary: Find source of basedatacombobox-based components with selectfirst="false" emits Debugger warnings New Features: Bugs Fixed: components with selectfirst="false" emits Debugger warnings Technical Reviewer: jcrowley QA Reviewer: max Doc Reviewer: (pending) Documentation: Release Notes: Details: Josh said: This is a workaround for an issue involving xpathQuery() calls from an oninit handler. Basedatacombobox kept getting "p is null" warnings because it was requesting data before the data was actually there, and because it doesn't itself have a datapath, but rather string attributes like "itemdatapath" that it uses in xpathQuery() calls, I couldn't just move things from oninit to ondata. --- Workaround: Check for data by querying the itemdatapath and if we don't get any results, install a delegate to listen for the "ondata"-event of the dataset. --- Josh said: The "p is null" errors are expected in situations where calls like this are made, but we shouldn't be generating them from our own base components. Other changes: Check for valid datapointer in "_updateSelectionByIndex", + nullpointer-check in the same method Removed a nondescriptive comment and an unnecessary debug-output. Tests: The example attached to the bug. Notice there are no "p is null" warnings. (You can compare with an unmodified branch, if you'd like.) Also, run: test/components/base/lzunit-basedatacombobox.lzx It operates as before, minus the "p is null" warnings. Modified: openlaszlo/trunk/lps/components/base/basedatacombobox.lzx =================================================================== --- openlaszlo/trunk/lps/components/base/basedatacombobox.lzx 2007-11-07 19:48:40 UTC (rev 7183) +++ openlaszlo/trunk/lps/components/base/basedatacombobox.lzx 2007-11-07 22:01:54 UTC (rev 7184) @@ -132,6 +132,10 @@ @keywords private --> <attribute name="_selectdel" value="null"/> + <!--- Data delegate. + @keywords private --> + <attribute name="_datacatchdel" value="null" /> + <!--- Called when combobox opens or closes --> <event name="onisopen"/> @@ -140,11 +144,8 @@ super.init(); if ( this.value == null && this.defaulttext != null ) { this._cbtext.setText(this.defaulttext); - } else if (this.value == null && selectfirst) { - // set first to be the initvalue - _updateSelectionByIndex(0,false,true); } else { - _updateSelectionByIndex(this._selectedIndex, true); + this._updateSelectionOnData(); } if ( this.statictext != null ) { @@ -152,6 +153,52 @@ } ]]> </method> + + <!--- @keywords private --> + <method name="destroy" ><![CDATA[ + if (this._datacatchdel != null) { + this._datacatchdel.unregisterAll(); + } + + super.destroy.apply(this, arguments); + ]]></method> + + <!--- This catches the initial ondata, since there is no datapath + set for a basedatacombobox, and itemdatapath won't receive + an ondata event. This fixes an issue with 'p is null' on + an init call to an xpathQuery(), which subsequently fixes + an issue where selectfirst="true" wasn't working for remote + data. + @keywords private --> + <method name="_updateSelectionOnData" ><![CDATA[ + var dp = new LzDatapointer(this); + + var arr = dp.xpathQuery(this.itemdatapath); + if (arr != null) { + if (this._datacatchdel != null) { + this._datacatchdel.unregisterAll(); + } + + if (this.value == null && selectfirst) { + // set first to be the initvalue + this._updateSelectionByIndex(0,false,true); + } else { + this._updateSelectionByIndex(this._selectedIndex, true); + } + } else { + if (this._datacatchdel == null) { + var itmpath = this.itemdatapath; + var iof = itmpath.indexOf( ":/" ); + if (iof > -1) { + var dset = dp.xpathQuery( itmpath.substring(0, iof + 2) ); + if (dset != null) { + this._datacatchdel = new LzDelegate(this, "_updateSelectionOnData", dset, "ondata"); + } + } + } + } + dp.destroy(); + ]]></method> <!--- @keywords private --> <handler name="ondata"> <![CDATA[ @@ -195,9 +242,14 @@ if (! (nodes instanceof Array)) nodes = [nodes]; dp.setPointer(nodes[index]); - var t = dp.xpathQuery(this.textdatapath); + var dpValid = dp.isValid(); + if (dpValid) { + var t = dp.xpathQuery(this.textdatapath); + } else { + var t = null; + } // if t is null, use default text (if it exists) - if( ( t == null || t.length == 0 ) && this.defaulttext && this.defaulttext['length'] > 0 ) + if( ( t == null || t.length == 0 ) && (this.defaulttext && this.defaulttext.length > 0) ) t = this.defaulttext; if ( this._cbtext && (this.statictext == null) ) { @@ -207,7 +259,11 @@ if ( this.selected && dp['p'] == this.selected['p'] ) return; if (! (dontSetValue || this.ismenu)) { - var v = dp.xpathQuery(this.valuedatapath); + if (dpValid) { + var v = dp.xpathQuery(this.valuedatapath); + } else { + var v = null; + } this.setValue(v,true, true); } this.setAttribute('text', t); @@ -268,12 +324,9 @@ cblist.setAttachTarget(this); cblist.setAttribute('shownitems', this.shownitems); - //local-dataset-modification by senshi var itd = this.itemdatapath; if (itd.indexOf( "local:" ) == 0) { itd = "local:" + "parent.owner." + itd.substring( 6 ); - if ($debug) - Debug.write( "local-dataset-modification:" + itd ); } cblist.item.setDatapath(itd); |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||