History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: LPP-4704
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: P0 P0
Assignee: Mamye Kratt
Reporter: Elliot Winard
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
OpenLaszlo

Find source of basedatacombobox-based components with selectfirst="false" emits Debugger warnings

Created: 11/Sep/07 09:15 AM   Updated: 09/Nov/07 04:58 PM
Component/s: Components - base
Affects Version/s: 4.0.5WaffleCone
Fix Version/s: RingDing (4.1)

Time Tracking:
Not Specified

File Attachments: 1. File datacombotest.lzx (3 kb)
2. XML File test-combobox-data.xml (0.8 kb)


Severity: Minor
Fixed in Change#: 7,184
Fixed in branch: trunk
Runtime: N/A
Flags: Products
Fix in hand: False


 Description  « Hide
STEPS:
1. create instance of component that extends basedatacombobox with attribute selectfirst="true"
2. look at debugger

RESULTS:
  debugger output -
WARNING: __LZgetNodes: p is null in LzDatapointer

EXPECTED:
no debugger output

 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
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.

Josh Crowley - 01/Nov/07 10:03 AM
This seems like it may be the result of (and, thus, duplicate of) LPP-5014.

The "p is null" thing is an intentional Debugger output (LPP-2760) for whenever an internal pointer is null, causing it to return "undefined". This occurring as a result of a component in the LFC is odd. I believe I've narrowed it down to the fact that oninit calls to xpathQuery() on remote data have been returning null, for some reason. (LPP-5014)

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.

Josh Crowley - 05/Nov/07 06:14 AM
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.

Josh Crowley - 05/Nov/07 11:21 AM
An example test case for the bug...

Josh Crowley - 05/Nov/07 11:22 AM
...and the data for the test case.

Josh Crowley - 09/Nov/07 04:58 PM
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:
LPP-4704 : Find source of basedatacombobox-based
   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);