Author Topic: JS: TreeBox onNodeUpdated: "node has no properties"  (Read 5041 times)

Offline David Serrano

  • PTeam Member
  • PixInsight Guru
  • ****
  • Posts: 503
JS: TreeBox onNodeUpdated: "node has no properties"
« on: 2007 September 05 12:08:28 »
This is "the dog bug report" ;).

When examining the different onNode* properties of the TreeBox object, I had to make some somersaults (spanish "voltereta" :P) to make onNodeUpdated work. Check out this code:

Code: [Select]
#include <pjsr/Sizer.jsh>
#include <pjsr/FrameStyle.jsh>
#include <pjsr/NumericControl.jsh>
#include <pjsr/TextAlign.jsh>
#include <pjsr/StdButton.jsh>
#include <pjsr/StdIcon.jsh>
#include <pjsr/ColorSpace.jsh>
#include <pjsr/SampleType.jsh>
#include <pjsr/UndoFlag.jsh>

function kappa_sigma_dialog() {
   this.__base__ = Dialog;
   this.__base__();

   this.target_List = new TreeBox (this);
   this.target_List.setMinSize (400, 200);
   this.target_List.headerVisible = true;
   this.target_List.setHeaderText (0, "Views");
   this.target_List.setHeaderAlignment (0, Align_Left);
   
   this.target_List.onNodeClicked = function() {
      var node = this.dialog.target_List.currentNode;
      console.writeln (node.text (0));
   }

   this.target_List.onNodeUpdated = function() {
      var node = this.dialog.target_List.currentNode;
      console.writeln (node.text (0));
//      var foo=0;
//      for (bar in node) if (!foo++) {     // beware of the dog!
//         console.writeln (node.text (0));
//      }
   }

   // Node creation helper
   function addViewNode (parent, view) {
      var node = new TreeBoxNode (parent);
      node.checkable = true;
      node.checked = false;

      node.setText (0, view.fullId);

      var image = view.image;
      var metadata = format ("%5d x %5d x %d", image.width, image.height, image.numberOfChannels);
      node.setText (1, metadata);

      return node;
   }

   // Build the view tree structure
   var windows = ImageWindow.windows;
   for (var i = 0; i < windows.length; ++i) {
      var node = addViewNode (this.target_List, windows[i].mainView);
      node.expanded = false; // or true to initially expand all preview lists

      var previews = windows[i].previews;
      for (var j = 0; j < previews.length; ++j)
         addViewNode (this.target_List, previews[j]);
   }
   this.target_List.sort();

}

kappa_sigma_dialog.prototype = new Dialog;
var dialog = new kappa_sigma_dialog();

dialog.execute();


There's a function onNodeClicked and another function onNodeUpdated. They are exactly the same, yet the code doesn't compile and triggers an error in line 28 (console.writeln in onNodeUpdated). Commenting that line out and uncommenting the "dog", it works as expected.

How did I reach that construct? I first tried:

Code: [Select]
for (bar in node) {
   if ("checked" == bar) {
      blah blah;
   }
}


But then I realized that all properties of node are accessible, no matter in which iteration of the loop we're in. So I simplified the if and added a foo variable so that we only go through the real code once. Maybe a little more obscure than the original I know.

---

Another bug, that I wasn't able to reproduce until now (at last! :)). I know I shouldn't be reporting two bugs in a single message, but I feel it's much easier in this case, because I'm reproducing it now with this code. We can call this the "cat" bug ;).

What would you think if you pasted that code just to see that it compiles and runs, against my claims? Well, just try again! One out of two times, it compiles and runs; the other it won't. But there's more: when it runs, the event handlers don't get called. Try to click here and there and see the console stay silent. Not even the Cancel button would close the dialog (of course, since its handler doesn't get called) (there's no such button in this example).

I've found this while building KSIntegration. Sometimes everything worked fine until I clicked in the interface element I was working on, then the interface stopped responding as I've just shown. I then added debug statements (console.writeln ("still here 1")) until I reached the point where the error was, then the code didn't compile and PixInsight informed me of the exact problem. I felt like I had to hunt the wrong line, just to have PixInsight tell me "Finally, you've found it! :P".

Now I've discovered that this happens without adding debug lines or anything. Just every second time.

I'm able to reproduce this by hitting F9 without explicitly saving the file. Maybe it doesn't show by saving or by using the mouse.
--
 David Serrano

Offline Juan Conejero

  • PTeam Member
  • PixInsight Jedi Grand Master
  • ********
  • Posts: 7111
    • http://pixinsight.com/
JS: TreeBox onNodeUpdated: "node has no properties"
« Reply #1 on: 2007 September 06 09:27:19 »
Let's have a look at the dog first:

Code: [Select]
#include <pjsr/Sizer.jsh>
#include <pjsr/FrameStyle.jsh>
#include <pjsr/NumericControl.jsh>
#include <pjsr/TextAlign.jsh>
#include <pjsr/StdButton.jsh>
#include <pjsr/StdIcon.jsh>
#include <pjsr/ColorSpace.jsh>
#include <pjsr/SampleType.jsh>
#include <pjsr/UndoFlag.jsh>

function kappa_sigma_dialog()
{
   this.__base__ = Dialog;
   this.__base__();

   this.target_List = new TreeBox( this );
   this.target_List.setMinSize( 400, 200 );
   this.target_List.headerVisible = true;
   this.target_List.setHeaderText( 0, "Views" );
   this.target_List.setHeaderAlignment( 0, Align_Left );

   // This is necessary for target_List to receive onNodeEntered events.
   // Comment it out to disable these events (which can be annoying).
   this.target_List.mouseTracking = true;

   this.target_List.onCurrentNodeUpdated = function( currentNode, oldCurrent )
   {
      /* Here we have discovered a bug: currentNode and oldCurrent are
         references to wrong TreeBoxNode objects -- this has been fixed */

      console.write( "current node updated" );
      // ### this code doesn't work due to the above bug:
      /*
      if ( currentNode )
         console.write( ": \'", currentNode.text( 0 ), "\'" );
      if ( oldCurrent )
         console.write( ", was: \'", oldCurrent.text( 0 ), "\'" );
      */
      // ### and this is a workaround, for now:
      if ( this.currentNode )
         console.write( ": \'", this.currentNode.text( 0 ), "\'" );
      console.writeln();
   }

   this.tellUs = function( what, node, column )
   {
      console.writeln( what, " \'", node.text( 0 ), " \' at column ", column );
   }

   this.target_List.onNodeActivated = function( node, column )
   {
      this.dialog.tellUs( "activated", node, column );
   }

   this.target_List.onNodeEntered = function( node, column )
   {
      this.dialog.tellUs( "entered", node, column );
   }
   
   this.target_List.onNodeClicked = function( node, column )
   {
      this.dialog.tellUs( "clicked", node, column );
   }

   this.target_List.onNodeDoubleClicked = function( node, column )
   {
      this.dialog.tellUs( "double clicked", node, column );
   }

   this.target_List.onNodeUpdated = function( node, column )
   {
      this.dialog.tellUs( "updated", node, column );
   }

   // Node creation helper
   function addViewNode( parent, view )
   {
      var node = new TreeBoxNode( parent );
      node.checkable = true;
      node.checked = false;

      node.setText( 0, view.fullId );

      var image = view.image;
      var metadata = format( "%5d x %5d x %d", image.width, image.height, image.numberOfChannels );
      node.setText( 1, metadata );

      return node;
   }

   // Build the view tree structure
   var windows = ImageWindow.windows;
   for ( var i = 0; i < windows.length; ++i )
   {
      var node = addViewNode( this.target_List, windows[i].mainView );
      node.expanded = false; // or true to initially expand all preview lists

      var previews = windows[i].previews;
      for ( var j = 0; j < previews.length; ++j )
         addViewNode (this.target_List, previews[j]);
   }
   this.target_List.sort();

   // ### WARNING ###
   // Not adding child controls to the sizer property is a nonstandard way to
   // lay out a child control on a dialog. It works (if there's only a child
   // control), but must be avoided.
   // So this is *mandatory*:
   this.sizer = new VerticalSizer( this ); // or HorizontalSizer; it's the same in this case
   this.sizer.add( this.target_List );

   this.windowTitle = "Dogs & Cats";
}

kappa_sigma_dialog.prototype = new Dialog;

console.show();

var dialog = new kappa_sigma_dialog();
dialog.execute();


This test script shows you the correct way to implement TreeBox event handlers. It also shows you the formal arguments that each handler receives (sorry this isn't documented elsewhere...).

The problem is that you were trying to access

Code: [Select]
this.dialog.target_List.currentNode

which in the context of a target_List's event handler is equivalent to:

Code: [Select]
this.currentNode

In theory, what you were trying to do should work, even if it's redundant. But here comes in the JavaScript compiler, which is an absolutely dynamic beast :) The problem is that the "this" variable inside an event handler isn't defined until the event handler gets invoked, and for some reason that I sincerely don't figure out, the compiler is not handling properly such a large indirection sequence. Perhaps a good candidate to be a reason for this is that I have embedded the SpiderMonkey engine taken from Firefox 2.0.0.5's source code, which implements JavaScript 1.7 that hasn't been officially released yet. I don't know. We're swimming in turbulent waters here...

Anyway, the "standard" way of doing things (the script above) works fine. Try it out.

And now the cat:

Quote
One out of two times, it compiles and runs; the other it won't. But there's more: when it runs, the event handlers don't get called. Try to click here and there and see the console stay silent. Not even the Cancel button would close the dialog (of course, since its handler doesn't get called)


Ok, I knew this. This is up to some degree a "normal" (watch out the quotes) behavior. What happens is that the error originates inside an event handler. This means that it is not being thrown from the script's root code path, but from a child code branch that runs asynchronously (because it is actually a callback routine) -- think in terms of a tree structure that represents the whole script code in RPN form, just as the structure of a XML document.

Exceptions generated in event handlers are not thrown immediately. Instead, they are stored as pendent exceptions. This is a peculiarity of JS's engine. Since a single JavaScript runtime object is created and reused by PixInsight to run successive scripts (to avoid expensive reinitializations), the runtime throws pendent exceptions before execution of a new script. This explains the cat :)

However, what is important is that you get the exception thrown with the correct error message and error location, even if you have to execute the script twice :) When this happens, don't forget to run the Execute > reset JavaScript Runtime command on the Script Editor. This will regenerate the whole engine from scratch.

Miau :mrgreen:
Juan Conejero
PixInsight Development Team
http://pixinsight.com/

Offline David Serrano

  • PTeam Member
  • PixInsight Guru
  • ****
  • Posts: 503
JS: TreeBox onNodeUpdated: "node has no properties"
« Reply #2 on: 2007 September 07 04:03:16 »
Quote from: "Juan Conejero"
This test script shows you the correct way to implement TreeBox event handlers. It also shows you the formal arguments that each handler receives


Thank you. Much nicer and cleaner code.


Quote from: "Juan Conejero"
The problem is that the "this" variable inside an event handler isn't defined until the event handler gets invoked, and for some reason that I sincerely don't figure out, the compiler is not handling properly such a large indirection sequence.


But that doesn't explain why that only happens in the onNodeUpdated handler and never in the other ones (I tried several, if not all of them).

Anyway, KSIntegration v0.5 will incorporate the proper parameters in the event handlers.

BTW Perl is also a dynamic one ;) ;) ;).
--
 David Serrano