FWD » Core Development » User Interface

A. When a Drag&Drop operation is initiated, an OLEStartDrag event is fired:
When the event is handled, the following statements are performed:
- get mouse coordinates (location point) relative to OCX window. This is done via 2 Win32 API calls GetCurrentPos and ScreenToClient.
- using mouse coordinates (x and y) dragNode is retrieved via a HitTest call:

ASSIGN chNode = COM-SELF:HitTest(x AS FLOAT, y AS FLOAT).
IF NOT chNode = 0 THEN
  DO:
    pchData:SetData(chNode:KEY). 
    ASSIGN gchDragNode = chNode.
  END.

REMARKS:
- Both, VB6 and the .NET TreeView controls expose the HitTest method (http://www.vbmigration.com/detknowledgebase.aspx?Id=125).
https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.treeview.hittest?redirectedfrom=MSDN&view=netframework-4.8#overloads
- chNode:KEY is stored in "cliboard" (SetData)
- COM-SELF system handle (Windows only) A component handle to the ActiveX object (ActiveX control or ActiveX automation object) that generated the event being handled by the currently executing ActiveX event procedure.
- Tree node is retrieved as a COM-HANDLE object.
- DragNode is cached in a global variable.
- Procedure parameter piAllowedEffects is not used.

B. While dragNode is dragged over OCX window a OLEDragOver event is fired:
- The code which handle this event retrieve current treeview node using the same technique, get mouse cursor location and then issue a HitTest.
- The treeview node the user is hovering over when draging is highlight: hTree:DropHighlight = chCurrNode.
- EnsureVisible is call on next and prevoius nodes: chNode:EnsureVisible. Ensures that the tree node is visible, expanding tree nodes and scrolling the tree view control as necessary.
REMARKS:
- Procedure parameters are not used when event is handled.
- EnsureVisible was removed in .NET

C. When Drag&Drop operation ends (dragged node is dropped) OLEDragDrop event is fired.
The ABL code which handle this event is more complex.
- Retrieve current node using the same technique, get mouse cursor location and then issue a HitTest.
- Test if current node is not the same with dragged node. No action is taken when dragging a node onto itself.
- Remove dragged node and add a new node with attributes extracted from dragged node.
- Some DB opertions are made for a table.
- Reset the DropHighLight node

   ASSIGN
     cKey = chDragNode:KEY
     cText = chDragNode:TEXT
     iNodeIndex = chDragNode:INDEX
     iTTIndex = INTEGER(ENTRY(1,chDragNode:KEY))
     iImage1 = chDragNode:IMAGE
     iImage2 = chDragNode:SelectedImage.

    hTree:Nodes:Remove(iNodeIndex). 
    chNewNode = hTree:Nodes:Add(chNode:INDEX, iRelation, cKey, cText, iImage1, iImage2).

  /* Reset the DropHighLight to be Nothing */
  ASSIGN 
    chNode = COM-SELF:HitTest(0 , 0).
    hTree:DropHighlight = chNode.

Good summary.

What happens if the drop event occurs outside of the tree control?

Greg Shah wrote:

Good summary.

What happens if the drop event occurs outside of the tree control?

If the drop event occurs outside of the tree control, HitTest returns an invalid drop node (0).

ASSIGN chNode = COM-SELF:HitTest(x AS FLOAT ,y AS FLOAT).

IF NOT chNode = 0 THEN
DO:
...
END. /* Valid chNode */

I created a new branch 4170a from current trunk and rebased with 3856b

An useful link to a description of VSFlexGrid Control: http://helpcentral.componentone.com/docs/vsflexgrid8/vsflexgridcontrol.htm
Descriptions of OLE Drag&Drop properties, methods and events for VSFlexGrid Control apply for all MS OCX's which support OLE Drag&Drop operations.

For this task I created a new branch 4170b from current trunk and I did a cherry-pick merge from branch 4170a which was created from an old branch 3856b

In manual OLE Drag&Drop the ABL code call HitTest on treeview. This method is not yet implemented in treeview, should I do that?

Yes, I think so.

Also there are a lot of Windows API calls how should these implemented?

Does the customer application use this manual mode? What APIs are used in that customer code?

Greg Shah wrote:

In manual OLE Drag&Drop the ABL code call HitTest on treeview. This method is not yet implemented in treeview, should I do that?

Yes, I think so.

Also there are a lot of Windows API calls how should these implemented?

Does the customer application use this manual mode? What APIs are used in that customer code?

PROCEDURE GetCursorPos EXTERNAL "user32":
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

Also, I found statements like: node = COM-SELF:HitTest(x AS FLOAT, x AS FLOAT).

is COM-SELF already implemented in FWD?

Here is another one: PROCEDURE ScreenToClient EXTERNAL "user32":

GetCursorPos and ScreenToClient are already implemented as native API emulation functions. Hynek can answer questions about the approach but you can see them in the NativeAPIEmulation class.

is COM-SELF already implemented in FWD?

I don't think so. We would need changes in variable_references.rules (see the portion inside the type == prog.sys_handle section). I guess the kw_self approach would be similar.

1. In the ABL code in one of our projects which use TreeView OCX and OLE Drag&Drop I found statements like:

chNewNode = hTree:Nodes:Add(chNode:INDEX, iRelation, cKey, cText, iImage1, iImage2).
hTree:Nodes:Remove(iNodeIndex). 

MS TreeView OCX (ITreeView) expose 2 COM interfaces INodes and INode. In ABL hTree:Nodes is the INodes interface.
Each node is a COM object which expose INode interface and allow ABL code to access node properties.
Example:

DEFINE VARIABLE gchDragNode     AS COM-HANDLE NO-UNDO.

ASSIGN
   cKey = chDragNode:KEY
   cText = chDragNode:TEXT
   iNodeIndex = chDragNode:INDEX
   iTTIndex = INTEGER(ENTRY(1,chDragNode:KEY))
   iImage1 = chDragNode:IMAGE
   iImage2 = chDragNode:SelectedImage.

The FWD TreeView implementation is not 100% compliant with MS TreeView OCX. The INodes interface is not exposed.
However equivalent methods are available directly:

   hTree:create-sub-node(iNodeId, chKey, chKey, true, img3, img3, newNodeId).
   hTree:remove-node(hNode:node-id).

2. For Drag&Drop I designed a COM wrapper for TreeView node. I designed and implemented a HitTest method which return a TreeView Node as a COM object
when hit and unknown comhandle when miss.

3. I found also statements like:
ASSIGN chNode = COM-SELF:HitTest(deX AS FLOAT , deY AS FLOAT).

The conversion rules for COM-SELF are not yet implemented in FWD. I'm working to do that but so far I didn't succeed.

4. Basically the implementation is almost done. Next I have to test a "manual drag&drop" operation and do the appropriate changes.

Marius Gligor wrote:

1. In the ABL code in one of our projects which use TreeView OCX and OLE Drag&Drop I found statements like:
[...]

MS TreeView OCX (ITreeView) expose 2 COM interfaces INodes and INode. In ABL hTree:Nodes is the INodes interface.
Each node is a COM object which expose INode interface and allow ABL code to access node properties.
Example:
[...]

The FWD TreeView implementation is not 100% compliant with MS TreeView OCX. The INodes interface is not exposed.
However equivalent methods are available directly:
[...]
I'm not sure if the original code can be converted directly without changes.

The MS Treeview part of the TREEVIEW interface originated during a POC phase of the customer's project. So there will be missing parts in the API for sure.

The basic object structure is there however. The "INodes" part is represented by TreeNodeCollection interface and its implementation and "INode" is represented by TreeNodeFace and the implemented classes.

I tried to use NativeApiEmulation in the following procedure extracted from the project.

PROCEDURE getMousePoint :
  DEFINE INPUT  PARAMETER pcObject AS HANDLE     NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeXpos  AS DECIMAL    NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeYpos  AS DECIMAL    NO-UNDO.

  DEFINE VARIABLE mpLong  AS MEMPTR     NO-UNDO.

  SET-SIZE(mpLong) = 8.

  RUN getCursorPos(INPUT mpLong).

  RUN screenToClient(INPUT pcObject:HWND, INPUT mpLong).

  ASSIGN 
    pdeXpos = DECIMAL(GET-LONG(mpLong,1))
    pdeYpos = DECIMAL(GET-LONG(mpLong,5))
   .
END PROCEDURE.

   /**
    * Converts screen (relative to display screen origin) location to client location - relative to the
    * window workspace origin.
    *
    * @param   widgetId
    *          Widget id. Top-level window of this widget will be used for the conversion.
    * @param   loc
    *          Screen location to convert.
    *
    * @return  Client location or {@code null} if the method fails.
    */
   @Override
   public NativePoint screenToClient(int widgetId, NativePoint loc)
   {
      Widget w = getWidget(widgetId);
      if (w == null)
      {
         return null;
      }

      GuiWindow wnd = (GuiWindow) w.parentOrSelf(GuiWindow.class);
      if (wnd == null)
      {
         return null;
      }

      NativePoint res = new NativePoint(0, 0);

      GuiDriver gd = (GuiDriver) getOutputManager().getInstanceDriver();
      gd.withSelectedWindow(wnd.getId().asInt(), () ->
      {
         res.translate(gd.getWindowLocation());
      });

      return res.minus(wnd.getWorkspaceScreenLocation());
   }

   /**
    * Converts screen (relative to display screen origin) location to client location - relative to the
    * window workspace origin.
    *
    * @param   widgetId
    *          Widget id. Top-level window of this widget will be used for the conversion.
    * @param   loc
    *          Screen location to convert.
    *
    * @return  Client location or {@code null} if the method fails.
    */
   @Override
   public NativePoint screenToClient(int widgetId, NativePoint loc)
   {
      Widget w = getWidget(widgetId);
      if (w == null)
      {
         return null;
      }

      GuiWindow wnd = (GuiWindow) w.parentOrSelf(GuiWindow.class);
      if (wnd == null)
      {
         return null;
      }

      NativePoint res = new NativePoint(0, 0);

      GuiDriver gd = (GuiDriver) getOutputManager().getInstanceDriver();
      gd.withSelectedWindow(wnd.getId().asInt(), () ->
      {
         res.translate(gd.getWindowLocation());
      });

      return loc.minus(res);
   }

Marius Gligor wrote:

I fixed that as follow:
[...]

The call to getWorkspaceScreenLocation was removed. The converted location must be relative to window workspace.

From Marius:

The TreeView is complex and uses handle instead of com-handle for nodes and node widgets. This approach requires changes in original 4GL code because the original code is based on com-handle. Maybe it is better to provide a new implementation using using handle instead of com-handle. The main change is that HitTest should return a handle not a com-handle like in the original code and some other small changes. Also the 4GL code inside the testcase should be changed accordingly.

We need to support both the COM-HANDLE (OCX) and the HANDLE (4GL widget) use cases. The same widget is meant to work for both. Existing code will most often use the COM-HANDLE approach and it should convert automatically. New code will be written by 4GL developers, using the 4GL widget (as a FWD extension) directly. Both must work.

The implementation of OLE Drag&Drop for TreeView is done and ready for a first code review.
1. What I did:
I provided an implementation which works with both handle and com-handle for TreeView node access.
Basically, I provided two HitTest methods, one which return a node of type handle and the other return a node as a comhandle.
I provided also 2 UAST tests available in testcases project as follow:
- uast/oledragdrop/treeview.p which use a handle type for node access.
- uast/oledragdrop/treeview1.p which use a comhandle type for node access.
Both tests are equivalent and works properly.

2. What remains to do:
- COM-SELF conversion. According to the ABL documentation:
A component handle to the ActiveX object (ActiveX control or ActiveX automation object) that generated the event being handled by the currently executing ActiveX event procedure.
the COM-SELF should occur in the body of an event handler procedure and in FWD should be converted into a widget handle, the replacement widget for OCX component.
- Manual mode of OLE Drag&Drop is driven by the user ABL code which handle OLE Drag&Drop events. Depending on that code, the missing TreeView features should be implemented as well.
For example in the ABL code of one project I found:

  chNode = hTree:Nodes:Add(chNode:INDEX, iRelation, cKey, cText, iImage1, iImage2).
  hTree:DropHighlight = chNode.
  chNode:SelectedImage.

The implementation code for TreeView OLE Drag&Drop is in the branch 4170b rev 11343-11360

I have reviewed the 4170b changes comparing to trunk and do not see the dangerous points. But the better way is to test I think.

So my plan is to prepare standalone sandbox, making the merge 4170b into 4335a. Then do the <large_customer_app> conversion and regression testing. If everything is OK - we can safely do the actual merging.

Is this plan OK?

Yes, great!

Greg Shah wrote:

Yes, great!

OK. I have just finished to prepare merged mix 4335a_v11421+4170b_11360. 30 source/rule files are affected in total. Now starting to do conversion and runtime tests for <large_customer_app>.

Eugenie Lyzenko wrote:

Greg Shah wrote:

Yes, great!

OK. I have just finished to prepare merged mix 4335a_v11421+4170b_11360. 30 source/rule files are affected in total. Now starting to do conversion and runtime tests for <large_customer_app>.

Greg,

I have finished the testing in big app. Everything looks good, no regressions I see.

So we can integrate 4170b into 4335a if it is fine. What I need is some time for 4335a be free from other updates. There are a couple of files and I just want the merge to be trouble free.

OK, go ahead when you are ready.

Greg Shah wrote:

OK, go ahead when you are ready.

The branch 4170b has been merged into 4335a rev 11425.

I see many new keywords added related to node and nodes. Were they a result of this issue? I noticed that node-expanded is in there with keyword KW_N_EXPED and KW_NODE_EXP. The KW_N_EXPED does not appear in the rules folder, and only in progress.g, so it may be a typo of the keyword, or it may just have been left behind as superfluous.

         new Keyword("node-bold"                      ,  0, KW_NODE_BLD, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-collapsed"                 ,  0, KW_N_COLED , false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-collapsing"                ,  0, KW_N_COLING, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-expanded"                  ,  0, KW_N_EXPED , false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-expanding"                 ,  0, KW_N_EXPING, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-count"                     ,  0, KW_NODE_CNT, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-expanded"                  ,  0, KW_NODE_EXP, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-height"                    ,  0, KW_N_HEIGHT, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-icon"                      ,  0, KW_NODE_ICO, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-id"                        ,  0, KW_NODE_ID , false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-idx"                       ,  0, KW_NODE_IDX, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-key"                       ,  0, KW_NODE_KEY, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-key-to-id"                 ,  0, KW_N_KEY_ID, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-parent"                    ,  0, KW_NODE_PAR, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-text"                      ,  0, KW_NODE_TXT, false),  // FWD extension, not a real 4GL keyword
         new Keyword("node-value"                     ,  0, KW_NODE_VAL, false),
         new Keyword("node-value-to-memptr"           ,  0, KW_NODE_V2M, false),
         new Keyword("node-value-to-longchar"         ,  0, KW_NODV_2LC, false),  // missing from keyword index and UNTESTED at this time
         new Keyword("nodes"                          ,  0, KW_NODES,    false ),  // FWD extension, not a real 4GL keyword

Roger Borrello wrote:

I see many new keywords added related to node and nodes. Were they a result of this issue? I noticed that node-expanded is in there with keyword KW_N_EXPED and KW_NODE_EXP.

Yes this seems like a duplicate. I will take care of this. Good catch!

On my windows image I have two versions of a tree view control: Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx and Microsoft TreeView Control version 5 (SP2) from comctl32.ocx. What version must be used for 4GL tests? In our repository testcases/uast I found only testcases/uast/oledragdrop/treeview.p test case that could not be run on 4GL. Are there real test cases of TreeView that can be executed on 4GL?

I assume v6 is the correct one, but I do not know for sure.

What 4GL create node methods are supported by TreeView FWD widget? I tried this code that works with 4GL

PROCEDURE initialize-controls :

hNode = chCtrlFrame:TreeView:NODES:ADD().
hNode:TEXT = "Label1".
hNode:KEY = "Key1".

hChildNode = chCtrlFrame:TreeView:NODES:ADD().
hChildNode:TEXT = "Label2".
hChildNode:KEY = "Key2".
hChildNode:PARENT = hNode.

hNode = chCtrlFrame:TreeView:NODES:ADD().
hNode:TEXT = "Label3".
hNode:KEY = "Key3".

MESSAGE hChildNode:FullPath " " hChildNode:PARENT:FullPath " #(all nodes) = " chCtrlFrame:TreeView:NODES:COUNT.

END PROCEDURE.

Also I found that Progress 4GL COM Object Viewer returns incorrect signature for Expand event of ITreeViewEvents when it introspects mscomctl.ocx:

PROCEDURE <event-proc-prefix>.Expand : 
    DEFINE INPUT-OUTPUT PARAMETER Node AS COM-HANDLE.  /* INode */
END PROCEDURE.

PROCEDURE CtrlFrame.TreeView.Expand : 
    DEFINE INPUT PARAMETER Node AS COM-HANDLE.  /* INode */
    MESSAGE Node:FullPath.
    COM-SELF:StartLabelEdit().
END PROCEDURE.

Sergey Ivanovskiy wrote:

What 4GL create node methods are supported by TreeView FWD widget? I tried this code that works with 4GL
[...]
but ADD and COUNT are not supported by FWD ocx conversion rules.

OCX conversion rule can't convert these methods because the add and getCount Java methods are not declared in TreeNodeCollection class.

Can I use these changes to add COUNT

=== modified file 'rules/annotations/ocx_conversion.rules'
--- rules/annotations/ocx_conversion.rules    2020-03-28 20:49:49 +0000
+++ rules/annotations/ocx_conversion.rules    2020-04-22 20:33:28 +0000
@@ -292,6 +292,7 @@
          <action>map2.put("createcellicon"        , "createImage")</action>
          <action>map2.put("allowdragdropothertree", "DragDropOtherTree")</action>
          <action>map2.put("createsubnode2"        , "createSubNode")</action>
+         <action>map2.put("count"                 , "getNodeCount")</action>
          <action>aliasMap.put("TREEVIEW"          , map2)</action>

          <!-- TREELIST -->

[ Com-Handle-Var = ] <com-handle>: Add ( 
      <anytype>-Relative BY-VARIANT-POINTER,
      <anytype>-Relationship BY-VARIANT-POINTER,
      <anytype>-Key BY-VARIANT-POINTER,
      <anytype>-Text BY-VARIANT-POINTER,
      <anytype>-Image BY-VARIANT-POINTER,
      <anytype>-SelectedImage BY-VARIANT-POINTER ). 

Sergey Ivanovskiy wrote:

Can I use these changes to add COUNT
[...]

If the new alias works, it is only a happy accident, it really shouldn't. The new alias you add is for treeview widget, but the needed attribute is for tree node collection resource. Better, to avoid problems in the future, add the method getCount in TreeNodeCollection.

I don't know how to map ADD method because it works for me only without parameters but COM Object viewer defines this method with its 6 optional parameters as
[...]

Just add add method to TreeNodeCollection with all needed overloads. Use only BDT types for the parameters.

Thank you, it is clear now. It seems that this tree implementation from Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx can have different nodes with the same keys. A key of a node is not a unique identifier, but its index is a unique identifier. I will add these new methods:

   /**
    * Returns the count of all the items in this collection.
    *
    * @return   The number of all nodes in this collection
    */
   default integer getCount()
   {
      return getNodeCount();
   }

   /**
    * Adds a new tree node as a first, last, next, previous or child node relative to the anchor
    * node given by its index.
    * 
    * @param    anchorIndex
    *           The anchor node index 
    * @param    relationType
    *           The relation type: first(0), last(1), next(2), previous(3) and child(4)
    * @param    key
    *           The key of the new node
    * @param    text
    *           The label of the new node
    * 
    * @return   The handle to the new node.
    */
   @ResourceType(type = TreeNodeFace.class)
   handle add(NumberType anchorIndex, NumberType relationType, character key, character text);

There are unresolved issues:

• The code that assigns the result of applying chained methods to the control frame of the tree to the variable of COM-HANDLE type
  

  
  DEFINE VARIABLE CtrlFrame AS WIDGET-HANDLE NO-UNDO.
  DEFINE VARIABLE chCtrlFrame AS COMPONENT-HANDLE NO-UNDO.
  
  DEFINE VARIABLE hNode AS COM-HANDLE.
  
  .....................................
  
  hNode = chCtrlFrame:TreeView:NODES:ITEM(1).
  MESSAGE hNode:TEXT " " hNode:KEY.
  

  
  is converted to
  

     handle hTreeView = TypeFactory.handle();
  
     comhandle hNode = UndoableFactory.comhandle();
  
  ..........................................................
  
           hNode.assign(hTreeView.unwrapTreeView().getNodes().unwrapTreeNodeCollection().getItem(new integer(1)));
           message(new Object[]
           {
              hNode.getProperty("TEXT"),
              " ",
              hNode.getProperty("KEY")
           });
  

  
  This conversion looks incorrect. The 4GL code works properly only if only hNode variable is defined as COM-HANDLE type but it doesn't work if it is defined as HANDLE type.

Is there a conceptual way to solve the issue in #4170-40? How these indirect COM objects should be supported? Should it be special rules from rules/annotations/ocx_conversion.rules that use type of the COM object (TreeFaceNode) and resolve hNode as hNode.asHandle().unwrapTreeFaceNode()?

Sergey Ivanovskiy wrote:

There are unresolved issues:

• The code that assigns the result of applying chained methods to the control frame of the tree to the variable of COM-HANDLE type
  [...]
  is converted to
  [...]
  This conversion looks incorrect. The 4GL code works properly only if only hNode variable is defined as COM-HANDLE type but it doesn't work if it is defined as HANDLE type.

You need to define hNode as com-handle and let ocx_conversion.rules know the expected type of the handle. Add the following to your ext-hints file:

<ocx com-handle="chNode" target-widget="com.goldencode.p2j.ui.TreeNodeFace" target-handle="hNode"/>

Thank you, it resolves the issue. Each node has KEY property and this property doesn't identify the tree node. Should we map it to NODE-VALUE property or to add new property with "KEY" name to TreeNodeFace and its implementations?

Sergey Ivanovskiy wrote:

Thank you, it resolves the issue. Each node has KEY property and this property doesn't identify the tree node. Should we map it to NODE-VALUE property or to add new property with "KEY" name to TreeNodeFace and its implementations?

There is a getNodeKey method in TreeNodeFace. Change the method name to getKey, this will fix make the ocx conversion rules correctly resolve the method.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Thank you, it resolves the issue. Each node has KEY property and this property doesn't identify the tree node. Should we map it to NODE-VALUE property or to add new property with "KEY" name to TreeNodeFace and its implementations?

There is a getNodeKey method in TreeNodeFace. Change the method name to getKey, this will fix make the ocx conversion rules correctly resolve the method.

If the current implementation doesn't behave as the original control, it must be fixed.

Hynek Cihlar wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Thank you, it resolves the issue. Each node has KEY property and this property doesn't identify the tree node. Should we map it to NODE-VALUE property or to add new property with "KEY" name to TreeNodeFace and its implementations?

There is a getNodeKey method in TreeNodeFace. Change the method name to getKey, this will fix make the ocx conversion rules correctly resolve the method.

If the current implementation doesn't behave as the original control, it must be fixed.

The key in FWD implementation should be unique because it is used by internal logic using nodesByKey map in TreeWidgetBase. Could we leave this implementation now and make KEY property be like NODE-VALUE?

   /**
    * Defines a getter for the KEY property of a tree node. Its value is not a unique identifier
    * and this property is a synonym of NODE-VALUE property.
    *
    * @return   The node key.
    */
   @LegacyAttribute(name = "KEY")
   default character getKey()
   {
      return getTreeNodeValue();
   }

   /**
    * Defines a setter for the KEY property of a tree node. This key is not a unique identifier
    * and is mapped into NODE-VALUE property.
    *
    * @param   value
    *          the new attribute value.
    */
   @LegacyAttribute(name = "KEY", setter = true)
   default void setKey(character value)
   {
      setTreeNodeValue(value);
   }

Another question is that

   /**
    * Returns the node at the specified index.
    *
    * @param   idx
    *          0-based index.
    *
    * @return  The node or {@code unknown} when the invalid index is provided.
    */
   @Override
   public handle getItem(NumberType idx)
   {
      return getTreeNode(idx);
   }

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

If the current implementation doesn't behave as the original control, it must be fixed.

The key in FWD implementation should be unique because it is used by internal logic using nodesByKey map in TreeWidgetBase. Could we leave this implementation now and make KEY property be like NODE-VALUE?

Does it mean the Add method in the original COM control accepts same key values at the same level? What are the key values of such nodes? Is there a method that can query nodes by key value? Which node is returned when you use the duplicate key value?

[...]

Another question is that

[...]
states that idx is zero based index. I tested that in 4GL it is a 1-based. So chCtrlFrame:TreeView:NODES:ITEM(1) accesses to the first element and chCtrlFrame:TreeView:NODES:ITEM(0) throws a 4GL exception: Error occured while accessing component property/method: Item. Index Out of bounds.

If the new method getItem needs to be 1-based, then make it so. Please make sure the javadocs are clear about that.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

If the current implementation doesn't behave as the original control, it must be fixed.

The key in FWD implementation should be unique because it is used by internal logic using nodesByKey map in TreeWidgetBase. Could we leave this implementation now and make KEY property be like NODE-VALUE?

Does it mean the Add method in the original COM control accepts same key values at the same level? What are the key values of such nodes?
Is there a method that can query nodes by key value? Which node is returned when you use the duplicate key value?

I don't know such methods in INodes interface.
Yes. The default ADD method (without any parameters) creates a new node at the end of the tree with KEY and TEXT properties equal to the " " space value. Then it is possible to change these values. TEXT is a label and KEY defines a node value. If ADD is used with parameters it behaves similarly according to these parameters:

   /**
    * Adds a new tree node as a first, last, next, previous or child node relative to the anchor
    * node given by its index.
    * 
    * @param    anchorIndex
    *           The anchor node index 
    * @param    relationType
    *           The relation type: first(0), last(1), next(2), previous(3) and child(4)
    * @param    key
    *           The key of the new node
    * @param    text
    *           The label of the new node
    * 
    * @return   The handle to the new node.
    */
   @ResourceType(type = TreeNodeFace.class)
   handle add(NumberType anchorIndex, NumberType relationType, character key, character text);

[ Com-Handle-Var = ] <com-handle>: Add ( 
      <anytype>-Relative BY-VARIANT-POINTER,
      <anytype>-Relationship BY-VARIANT-POINTER,
      <anytype>-Key BY-VARIANT-POINTER,
      <anytype>-Text BY-VARIANT-POINTER,
      <anytype>-Image BY-VARIANT-POINTER,
      <anytype>-SelectedImage BY-VARIANT-POINTER ). 

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

If the current implementation doesn't behave as the original control, it must be fixed.

The key in FWD implementation should be unique because it is used by internal logic using nodesByKey map in TreeWidgetBase. Could we leave this implementation now and make KEY property be like NODE-VALUE?

Does it mean the Add method in the original COM control accepts same key values at the same level? What are the key values of such nodes?

Yes. The default ADD method (without any parameters) creates a new node at the end of the tree with KEY and TEXT properties equal to the " " space value. Then it is possible to change these values. TEXT is a label and KEY defines a node value.it If ADD is used with parameters it behaves similarly according to these parameters:

How does Add behave when you call it multiple times without any parameters? What the key values will be? What will the FullPath return for these nodes?

Nodes of the Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx has Index property that is used as a node key, but a tree node Key property can hold the same value for different nodes. NODES property of the tree widget holds all hierarchy of tree nodes. FWD implementation is different now and it can be an issue because the value of Index property is not an index within siblings tree node but it is a global node identifier. I can fix it if you have no objections.

Hynek Cihlar wrote:

How does Add behave when you call it multiple times without any parameters? What the key values will be? What will the FullPath return for these nodes?

All nodes are added to the root node as a last node within the sibling hierarchy attached to the root. As I wrote in #4170-48 all added nodes have the same value that is equal to " ". FullPath is just a string build from labels separated by a backslash \ in the path from the root node to the target node.

Please review the committed revision 11489 (4231b). It still doesn't work according to the Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx because it needs to fix #4170-50.

Sergey Ivanovskiy wrote:

Nodes of the Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx has Index property that is used as a node key, but a tree node Key property can hold the same value for different nodes. NODES property of the tree widget holds all hierarchy of tree nodes. FWD implementation is different now and it can be an issue because the value of Index property is not an index within siblings tree node but it is a global node identifier. I can fix it if you have no objections.

No objections. Just beware that the help text in the type library for INode:Key property states "Returns/sets the unique string of an object in a collection.".

Sergey Ivanovskiy wrote:

Please review the committed revision 11489 (4231b). It still doesn't work according to the Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx because it needs to fix #4170-50.

Code review 4231b revision 11489.

I think we can safely remove getNodeCount and only declare getCount (with LegacyAttribute(name = "NODE-COUNT")) in TreeNodeCollection.

throwError has incorrect location in the class, as private it should follow the protected methods.

If getCount returns the count recursively including, shouldn't add also take a recursive index?

I think the mapping of getKey to getTreeNodeValue makes sense. Its name however collides with TreeNodeResource.key and TreeNodeFace.getNodeKey. We should rename TreeNodeResource.key to TreeNodeResource.uniqueKey (or similar), to make these two distinct. Or map getKey and setKey to getNodeValue and setNodeValue already during conversion.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Please review the committed revision 11489 (4231b). It still doesn't work according to the Microsoft TreeView Control 6.0 (SP6) from mscomctl.ocx because it needs to fix #4170-50.

Code review 4231b revision 11489.

I think we can safely remove getNodeCount and only declare getCount (with LegacyAttribute(name = "NODE-COUNT")) in TreeNodeCollection.

throwError has incorrect location in the class, as private it should follow the protected methods.

If getCount returns the count recursively including, shouldn't add also take a recursive index?

Yes, agree. Please review the committed revision 11491 that fixed getItem() and add() methods of TreeNodeCollectionResource to be able to search through all tree nodes. Now the FWD produces the same tree structure as 4GL does according to invocations of add().

I think the mapping of getKey to getTreeNodeValue makes sense. Its name however collides with TreeNodeResource.key and TreeNodeFace.getNodeKey. We should rename TreeNodeResource.key to TreeNodeResource.uniqueKey (or similar), to make these two distinct. Or map getKey and setKey to getNodeValue and setNodeValue already during conversion.

Working to fix it.

Sergey,

The regression starting in 11495 does not allow the 4231b branch to be compiled.

Please fix this one, otherwise we have no ability to keep up to date working branch version and safely commit the new changes and even start new conversion.

...
[ant:javac] /...../p2j/src/com/goldencode/p2j/util/HandleCommon.java:146: error: types TreeNodeFace and CommonWidget are incompatible; both define getParent(), but with unrelated return types
[ant:javac] public interface HandleCommon
[ant:javac]        ^
...
[ant:javac] 1 error
[ant:javac] 18 warnings
...

Sorry, I missed this issue due to incremental builds. Please update to rev 11499 (4231b).

Hynek, please review the committed rev 11500 (4231b) that removed already defined by TreeFaceNode methods and added alias map for KEY, INDEX, PARENT and TEXT properties.

Code review 4231b revision 11491.

TreeNodeCollectionResource.add():
Before the change the method raises out of bound error only when the passed in relation is relative to the passed in anchor node. Was this change intentional?

TreeNodeCollectionResource.createNode and TNCR.throwError are at wrong file location. These should be moved among the private methods.

I think the parameter key in TNCR.add should be named nodeValue since with the change the key-value mapping happens at conversion time.

Beware that TNF.getNodeId may not be successive. Consider the case you add a node, remove it and then add another one. I mention this because you map index property to nodeId.

Hynek Cihlar wrote:

Code review 4231b revision 11491.

TreeNodeCollectionResource.add():
Before the change the method raises out of bound error only when the passed in relation is relative to the passed in anchor node. Was this change intentional?

Yes. I tested these cases.

TreeNodeCollectionResource.createNode and TNCR.throwError are at wrong file location. These should be moved among the private methods.

I think the parameter key in TNCR.add should be named nodeValue since with the change the key-value mapping happens at conversion time.

Ok.

Beware that TNF.getNodeId may not be successive. Consider the case you add a node, remove it and then add another one. I mention this because you map index property to nodeId.

I will test it more thoroughly. It seems that INDEX property is not successive in this case. If INDEX is not the same as NODE-ID, then it needs to add new property to the node and to implement additional logic.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Code review 4231b revision 11491.

TreeNodeCollectionResource.add():
Before the change the method raises out of bound error only when the passed in relation is relative to the passed in anchor node. Was this change intentional?

Yes. I tested these cases.

So you say valid index must be provided even when adding the node as first or last child?

Yes, if we use add(...) (chCtrlFrame:TreeView:NODES:ADD(1, 4, "Key2", "Label2")) with incorrect parameters, then 4GL throws these errors. To add a first node to the tree I used add() without parameters.

Hynek, could you help to understand these code

         <!-- TREEVIEW event handlers -->
         <rule>cbName.endsWith(".ONEXPANDING")
            <action>lref = execLib("doTrigger", ref.parent, cbSymbol, "NODE-EXPANDING", prog.kw_n_exping, thName, thRefid.get(thName.toUpperCase()), true)</action>
            <action>ref1 = lref.get(0)</action>
            <action>ref = execLib("doDefineVar", ref1, "hNode", prog.var_handle, "handle", prog.kw_handle)</action>
            <action>execLib("doAssignAttr", ref1, "hNode", prog.var_handle, ref.id, thName, thRefid.get(thName.toUpperCase()), "trigger-node", prog.attr_handle, prog.kw_trig_nod)</action>
            <action>ref1 = lref.get(1)</action>
            <action>execLib("doObjMethodArgument", ref1, ref, "hNode", "node-id", prog.attr_int, prog.kw_node_id, true)</action>
            <action>execLib("doObjMethodArgument", ref1, ref, "hNode", "node-key", prog.attr_char, prog.kw_node_key, true)</action>
         </rule>

         <rule>cbName.endsWith(".ONEXPANDED")
            <action>lref = execLib("doTrigger", ref.parent, cbSymbol, "NODE-EXPANDED", prog.kw_n_exping, thName, thRefid.get(thName.toUpperCase()), false)</action>
            <action>ref1 = lref.get(0)</action>
            <action>ref = execLib("doDefineVar", ref1, "hNode", prog.var_handle, "handle", prog.kw_handle)</action>
            <action>execLib("doAssignAttr", ref1, "hNode", prog.var_handle, ref.id, thName, thRefid.get(thName.toUpperCase()), "trigger-node", prog.attr_handle, prog.kw_trig_nod)</action>
            <action>ref1 = lref.get(1)</action>
            <action>execLib("doObjMethodArgument", ref1, ref, "hNode", "node-key", prog.attr_char, prog.kw_node_key, false)</action>
         </rule>

         <rule>cbName.endsWith(".ONCOLLAPSED")
            <action>lref = execLib("doTrigger", ref.parent, cbSymbol, "NODE-COLLAPSED", prog.kw_n_coled, thName, thRefid.get(thName.toUpperCase()), false)</action>
            <action>ref1 = lref.get(0)</action>
            <action>ref = execLib("doDefineVar", ref1, "hNode", prog.var_handle, "handle", prog.kw_handle)</action>
            <action>execLib("doAssignAttr", ref1, "hNode", prog.var_handle, ref.id, thName, thRefid.get(thName.toUpperCase()), "trigger-node", prog.attr_handle, prog.kw_trig_nod)</action>
            <action>ref1 = lref.get(1)</action>
            <action>execLib("doObjMethodArgument", ref1, ref, "hNode", "node-id", prog.attr_int, prog.kw_node_id, false)</action>
            <action>execLib("doObjMethodArgument", ref1, ref, "hNode", "node-key", prog.attr_char, prog.kw_node_key, false)</action>
         </rule>

         <rule>cbName.endsWith(".ONCHANGENODE")
            <action>lref = execLib("doTrigger", ref.parent, cbSymbol, "VALUE-CHANGED", prog.kw_val_chg, thName, thRefid.get(thName.toUpperCase()), false)</action>
            <action>ref1 = lref.get(0)</action>
            <action>ref = execLib("doDefineVar", ref1, "hTrigNode", prog.var_handle, "handle", prog.kw_handle)</action>
            <action>execLib("doAssignAttr", ref1, "hNode", prog.var_handle, ref.id, thName, thRefid.get(thName.toUpperCase()), "trigger-node", prog.attr_handle, prog.kw_trig_nod)</action>
            <action>ref2 = execLib("doDefineVar", ref1, "hCurNode", prog.var_handle, "handle", prog.kw_handle)</action>
            <action>execLib("doAssignAttr", ref1, "hNode", prog.var_handle, ref2.id, thName, thRefid.get(thName.toUpperCase()), "selected-node", prog.attr_handle, prog.kw_sel_node)</action>
            <action>ref1 = lref.get(1)</action>
            <action>execLib("doObjMethodArgument", ref1, ref,  "hTrigNode", "node-key", prog.attr_char, prog.kw_node_key, false)</action>
            <action>execLib("doObjMethodArgument", ref1, ref,  "hTrigNode", "node-id",  prog.attr_int,  prog.kw_node_id,  false)</action>
            <action>execLib("doObjMethodArgument", ref1, ref2, "hCurNode",  "node-key", prog.attr_char, prog.kw_node_key, false)</action>
            <action>execLib("doObjMethodArgument", ref1, ref2, "hCurNode",  "node-id",  prog.attr_int,  prog.kw_node_id,  false)</action>
         </rule>

         <rule>cbName.endsWith(".ONCLICK")
            <action>execLib("doTrigger", ref.parent, cbSymbol, "LEFT-MOUSE-CLICK", prog.KW_LEFT_MC, thName, thRefid.get(thName.toUpperCase()), false)</action>
         </rule>

         <rule>cbName.endsWith(".ONDBLCLICK")
            <action>execLib("doTrigger", ref.parent, cbSymbol, "LEFT-MOUSE-DBLCLICK", prog.KW_LEFT_MDC, thName, thRefid.get(thName.toUpperCase()), false)</action>
         </rule>

         <rule>cbName.endsWith(".ONKEYDOWN")
            <action>lref = execLib("doTrigger", ref.parent, cbSymbol, "ANY-KEY", prog.kw_any_key, thName, thRefid.get(thName.toUpperCase()), false)</action>
            <action>ref1 = lref.get(0)</action>
            <action>ref = execLib("doDefineVar", ref1, "keyCode", prog.var_int, "integer", prog.kw_int)</action>

            <!-- keyCode = last-event:code. -->
            <action>ref2 = createProgressAst(prog.assignment, "assignment", null)</action>
            <action>ref1.graftAt(ref2, 0)</action>
            <action>ref2 = createProgressAst(prog.assign, "=", ref2)</action>
            <action>ref2 = createProgressAst(prog.var_int, "keyCode", ref2)</action>
            <action>ref2.putAnnotation("oldtype", #(long) prog.symbol)</action>
            <action>ref2.putAnnotation("refid", ref.id)</action>
            <action>ref2 = ref2.parent</action>
            <action>ref2 = createProgressAst(prog.expression, "expression", ref2)</action>
            <action>ref2.putAnnotation("winKeyCode", true)</action>
            <action>ref2 = createProgressAst(prog.colon, ":", ref2)</action>
            <action>ref2 = createProgressAst(prog.sys_handle, "last-event", ref2)</action>
            <action>ref2.putAnnotation("oldtype", #(long) prog.kw_last_evt)</action>
            <action>ref2 = ref2.parent</action>
            <action>ref2 = createProgressAst(prog.attr_int, "code", ref2)</action>
            <action>ref2.putAnnotation("oldtype", #(long) prog.kw_code)</action>

            <action>ref1 = lref.get(1)</action>

            <!-- keyCode argument -->
            <action>ref2 = createProgressAst(prog.parameter, "parameter", ref1)</action>
            <action>ref2 = createProgressAst(prog.expression, "expression", ref2)</action>
            <action>ref2 = createProgressAst(prog.var_int, "keyCode", ref2)</action>
            <action>ref2.putAnnotation("refid", ref.id)</action>

            <!-- false argument -->
            <action>ref2 = createProgressAst(prog.parameter, "parameter", ref1)</action>
            <action>ref2 = createProgressAst(prog.expression, "expression", ref2)</action>
            <action>ref2 = createProgressAst(prog.bool_false, "false", ref2)</action>
            <action>ref2.putAnnotation("is-literal", true)</action>

         </rule>

PROCEDURE <event-proc-prefix>.Expand : 
    DEFINE INPUT-OUTPUT PARAMETER Node AS COM-HANDLE.  /* INode */
END PROCEDURE.
PROCEDURE <event-proc-prefix>.Collapse : 
    DEFINE INPUT-OUTPUT PARAMETER Node AS COM-HANDLE.  /* INode */
END PROCEDURE.
PROCEDURE <event-proc-prefix>.AfterLabelEdit : 
    DEFINE INPUT-OUTPUT PARAMETER Cancel AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER NewString AS CHARACTER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.BeforeLabelEdit : 
    DEFINE INPUT-OUTPUT PARAMETER Cancel AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.Click : 
END PROCEDURE.
PROCEDURE <event-proc-prefix>.DblClick : 
END PROCEDURE.
PROCEDURE <event-proc-prefix>.KeyDown : 
    DEFINE INPUT-OUTPUT PARAMETER KeyCode AS INTEGER.
    DEFINE INPUT PARAMETER Shift AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.KeyPress : 
    DEFINE INPUT-OUTPUT PARAMETER KeyAscii AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.KeyUp : 
    DEFINE INPUT-OUTPUT PARAMETER KeyCode AS INTEGER.
    DEFINE INPUT PARAMETER Shift AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.MouseDown : 
    DEFINE INPUT PARAMETER Button AS INTEGER.
    DEFINE INPUT PARAMETER Shift AS INTEGER.
    DEFINE INPUT PARAMETER x AS INTEGER.
    DEFINE INPUT PARAMETER y AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.MouseMove : 
    DEFINE INPUT PARAMETER Button AS INTEGER.
    DEFINE INPUT PARAMETER Shift AS INTEGER.
    DEFINE INPUT PARAMETER x AS INTEGER.
    DEFINE INPUT PARAMETER y AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.MouseUp : 
    DEFINE INPUT PARAMETER Button AS INTEGER.
    DEFINE INPUT PARAMETER Shift AS INTEGER.
    DEFINE INPUT PARAMETER x AS INTEGER.
    DEFINE INPUT PARAMETER y AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.NodeCheck : 
    DEFINE INPUT-OUTPUT PARAMETER Node AS COM-HANDLE.  /* INode */
END PROCEDURE.
PROCEDURE <event-proc-prefix>.NodeClick : 
    DEFINE INPUT-OUTPUT PARAMETER Node AS COM-HANDLE.  /* INode */
END PROCEDURE.
PROCEDURE <event-proc-prefix>.OLECompleteDrag : 
    DEFINE INPUT-OUTPUT PARAMETER Effect AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.OLEDragDrop : 
    DEFINE INPUT-OUTPUT PARAMETER Data AS COM-HANDLE.  /* IVBDataObject */
    DEFINE INPUT-OUTPUT PARAMETER Effect AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER Button AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER Shift AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER x AS DECIMAL.
    DEFINE INPUT-OUTPUT PARAMETER y AS DECIMAL.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.OLEDragOver : 
    DEFINE INPUT-OUTPUT PARAMETER Data AS COM-HANDLE.  /* IVBDataObject */
    DEFINE INPUT-OUTPUT PARAMETER Effect AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER Button AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER Shift AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER x AS DECIMAL.
    DEFINE INPUT-OUTPUT PARAMETER y AS DECIMAL.
    DEFINE INPUT-OUTPUT PARAMETER State AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.OLEGiveFeedback : 
    DEFINE INPUT-OUTPUT PARAMETER Effect AS INTEGER.
    DEFINE INPUT-OUTPUT PARAMETER DefaultCursors AS LOGICAL.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.OLESetData : 
    DEFINE INPUT-OUTPUT PARAMETER Data AS COM-HANDLE.  /* IVBDataObject */
    DEFINE INPUT-OUTPUT PARAMETER DataFormat AS INTEGER.
END PROCEDURE.
PROCEDURE <event-proc-prefix>.OLEStartDrag : 
    DEFINE INPUT-OUTPUT PARAMETER Data AS COM-HANDLE.  /* IVBDataObject */
    DEFINE INPUT-OUTPUT PARAMETER AllowedEffects AS INTEGER.
END PROCEDURE.

Sergey Ivanovskiy wrote:

These events: ONEXPANDING, ONEXPANDED, ... are not documented and 4GL COM object viewer finds ITreeViewEvents with different set of event handlers in mscomctl.ocx:

The events in the rules file are not for the ms treeview control, but a customer treelist control.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Code review 4231b revision 11491.

TreeNodeCollectionResource.add():
Before the change the method raises out of bound error only when the passed in relation is relative to the passed in anchor node. Was this change intentional?

Yes. I tested these cases.

So you say valid index must be provided even when adding the node as first or last child?

We can use this construction chCtrlFrame:TreeView:NODES:ADD(?, 0, "Key0", "Label0"). (tvwFirst = 0) or chCtrlFrame:TreeView:NODES:ADD(?, 1, "Key0", "Label0"). (tvwLast = 1) for adding the first node in the tree. But this code chCtrlFrame:TreeView:NODES:ADD(1, 0, "Key0", "Label0"). (chCtrlFrame:TreeView:NODES:ADD(1, 1, "Key0", "Label0").) throws the index out of bounds error. Planning to change the code according to this test case and add new ones to testcases project.

I convinced myself that KEY property is not unique but it was incorrect. Actually the keys are unique but can be set after the node is created. It follows from tests.

Sergey Ivanovskiy wrote:

I convinced myself that KEY property is not unique but it was incorrect. Actually the keys are unique but can be set after the node is created. It follows from tests.

It happened due to the test case that empty key = '' (not space ' ') can be repeated for different nodes. It looks like a feature at least I can observe it with 4GL code:

hNode = chCtrlFrame:TreeView:NODES:ADD().
hNode:TEXT = "Label1".
hNode:KEY = "Key1".

hNode = chCtrlFrame:TreeView:NODES:ADD().
hNode:TEXT = "Label2".
hNode:KEY = "Key2".

hNode = chCtrlFrame:TreeView:NODES:ADD().

hNode = chCtrlFrame:TreeView:NODES:ADD().

hSelectedNode = chCtrlFrame:TreeView:NODES:ITEM(2).

ON CHOOSE OF BUTTON-3
DO:
hNode = hSelectedNode.

hNode:TEXT = hNode:TEXT + " with the same key".

hNode:KEY = "".

MESSAGE "Change key  Index = '" hNode:Index "' key = '" hNode:Key "' text = '" hNode:Text "'".

END.

PROCEDURE CtrlFrame.TreeView.NodeClick : 
   DEFINE INPUT PARAMETER Node AS COM-HANDLE.  /* INode */
   hSelectedNode = Node.
   MESSAGE "Key = '" Node:Key "' Index = '" Node:Index "'".
END PROCEDURE.

After removing the node all indices of tree nodes are rebuilt. So the INDEX is not NODE-ID.

Sergey,

What is your plans for implementing GET-NODE-AT(X,Y) tree view widget method?

Eugenie, I am not aware of this method. Could you help to find 4GL examples of GET-NODE-AT(X, Y)? I didn't find this method in

[
  uuid(c74190b4-8589-11d1-b16a-00c0f0283628),
  helpstring("Displays a hierarchical list of Node objects, each of which consists of a label and an optional bitmap."),
  helpcontext(0x00033528),
  hidden,
  dual,
  nonextensible
]
dispinterface ITreeView {
    methods:
        [restricted] 
        void _stdcall QueryInterface(
            [in] GUID#i* riid,
            [out] void** ppvObj
        );
        [restricted] 
        unsigned long _stdcall AddRef();
        [restricted] 
        unsigned long _stdcall Release();
        [restricted] 
        void _stdcall GetTypeInfoCount([out] unsigned int* pctinfo);
        [restricted] 
        void _stdcall GetTypeInfo(
            [in] unsigned int itinfo,
            [in] unsigned long lcid,
            [out] void** pptinfo
        );
        [restricted] 
        void _stdcall GetIDsOfNames(
            [in] GUID#i* riid,
            [in] char** rgszNames,
            [in] unsigned int cNames,
            [in] unsigned long lcid,
            [out] long* rgdispid
        );
        [restricted] 
        void _stdcall Invoke(
            [in] long dispidMember,
            [in] GUID#i* riid,
            [in] unsigned long lcid,
            [in] unsigned short wFlags,
            [in] DISPPARAMS#i* pdispparams,
            [out] VARIANT* pvarResult,
            [out] EXCEPINFO#i* pexcepinfo,
            [out] unsigned int* puArgErr
        );
        [id(0x00000001), propget, helpstring("Returns a reference to a Node or ListItem object and highlights the object with the system highlight color."), helpcontext(0x0003353e)] 
        INode#i* DropHighlight();
        [id(0x00000001), propputref, helpstring("Returns a reference to a Node or ListItem object and highlights the object with the system highlight color."), helpcontext(0x0003353e)] 
        void DropHighlight([in] INode#i* rhs);
        [id(0x00000001), propput, helpstring("Returns a reference to a Node or ListItem object and highlights the object with the system highlight color."), helpcontext(0x0003353e)] 
        void DropHighlight([in] VARIANT* rhs);
        [id(0x00000002), propget, helpstring("Determines whether the selected item will display as selected when the TreeView loses focus"), helpcontext(0x0003353f)] 
        VARIANT_BOOL HideSelection();
        [id(0x00000002), propput, helpstring("Determines whether the selected item will display as selected when the TreeView loses focus"), helpcontext(0x0003353f)] 
        void HideSelection([in] VARIANT_BOOL rhs);
        [id(0x00000003), propget, helpstring("Returns/sets the ImageList control to be used."), helpcontext(0x00033540)] 
        IDispatch* ImageList();
        [id(0x00000003), propputref, helpstring("Returns/sets the ImageList control to be used."), helpcontext(0x00033540)] 
        void ImageList([in] IDispatch* rhs);
        [id(0x00000003), propput, helpstring("Returns/sets the ImageList control to be used."), helpcontext(0x00033540)] 
        void ImageList([in] IDispatch* rhs);
        [id(0x00000004), propget, helpstring("Returns/sets the width of the indentation for a TreeView control."), helpcontext(0x00033541)] 
        float Indentation();
        [id(0x00000004), propput, helpstring("Returns/sets the width of the indentation for a TreeView control."), helpcontext(0x00033541)] 
        void Indentation([in] float rhs);
        [id(0x00000005), propget, helpstring("Returns/sets a value that determines if a user can edit the label of a ListItem or Node object."), helpcontext(0x00033542)] 
        LabelEditConstants#i LabelEdit();
        [id(0x00000005), propput, helpstring("Returns/sets a value that determines if a user can edit the label of a ListItem or Node object."), helpcontext(0x00033542)] 
        void LabelEdit([in] LabelEditConstants#i rhs);
        [id(0x00000006), propget, helpstring("Returns/sets the style of lines displayed between Node objects."), helpcontext(0x00033543)] 
        TreeLineStyleConstants#i LineStyle();
        [id(0x00000006), propput, helpstring("Returns/sets the style of lines displayed between Node objects."), helpcontext(0x00033543)] 
        void LineStyle([in] TreeLineStyleConstants#i rhs);
        [id(0x00000007), propget, helpstring("Returns/sets the type of mouse pointer displayed when over part of an object."), helpcontext(0x00033544)] 
        MousePointerConstants#i MousePointer();
        [id(0x00000007), propput, helpstring("Returns/sets the type of mouse pointer displayed when over part of an object."), helpcontext(0x00033544)] 
        void MousePointer([in] MousePointerConstants#i rhs);
        [id(0x00000008), propget, helpstring("Sets a custom mouse icon."), helpcontext(0x00033545)] 
        IPictureDisp#i* MouseIcon();
        [id(0x00000008), propput, helpstring("Sets a custom mouse icon."), helpcontext(0x00033545)] 
        void MouseIcon([in] IPictureDisp#i* rhs);
        [id(0x00000008), propputref, helpstring("Sets a custom mouse icon."), helpcontext(0x00033545)] 
        void MouseIcon([in] IPictureDisp#i* rhs);
        [id(0x00000009), propget, helpstring("Returns a reference to a collection of Node objects."), helpcontext(0x00033546)] 
        INodes#i* Nodes();
        [id(0x00000009), propputref, helpstring("Returns a reference to a collection of Node objects."), helpcontext(0x00033546)] 
        void Nodes([in] INodes#i* rhs);
        [id(0x0000000a), propget, helpstring("Returns/sets the delimiter string used for the path returned by the FullPath property."), helpcontext(0x00033547)] 
        BSTR PathSeparator();
        [id(0x0000000a), propput, helpstring("Returns/sets the delimiter string used for the path returned by the FullPath property."), helpcontext(0x00033547)] 
        void PathSeparator([in] BSTR rhs);
        [id(0x0000000b), propget, helpstring("Returns/sets a value which determines if a ListItem or Node object is selected."), helpcontext(0x00033548)] 
        INode#i* SelectedItem();
        [id(0x0000000b), propputref, helpstring("Returns/sets a value which determines if a ListItem or Node object is selected."), helpcontext(0x00033548)] 
        void SelectedItem([in] INode#i* rhs);
        [id(0x0000000b), propput, helpstring("Returns/sets a value which determines if a ListItem or Node object is selected."), helpcontext(0x00033548)] 
        void SelectedItem([in] VARIANT* rhs);
        [id(0x0000000c), propget, helpstring("Indicates whether the elements of a control are automatically sorted alphabetically."), helpcontext(0x00033549)] 
        VARIANT_BOOL Sorted();
        [id(0x0000000c), propput, helpstring("Indicates whether the elements of a control are automatically sorted alphabetically."), helpcontext(0x00033549)] 
        void Sorted([in] VARIANT_BOOL rhs);
        [id(0x0000000d), propget, helpstring("Displays a hierarchical list of Node objects, each of which consists of a label and an optional bitmap."), helpcontext(0x0003354a)] 
        TreeStyleConstants#i Style();
        [id(0x0000000d), propput, helpstring("Displays a hierarchical list of Node objects, each of which consists of a label and an optional bitmap."), helpcontext(0x0003354a)] 
        void Style([in] TreeStyleConstants#i rhs);
        [id(0x0000060e), propget, helpstring("Returns/Sets whether this control can act as an OLE drag/drop source, and whether this process is started automatically or under programmatic control."), helpcontext(0x00033691)] 
        OLEDragConstants#i OLEDragMode();
        [id(0x0000060e), propput, helpstring("Returns/Sets whether this control can act as an OLE drag/drop source, and whether this process is started automatically or under programmatic control."), helpcontext(0x00033691)] 
        void OLEDragMode([in] OLEDragConstants#i rhs);
        [id(0x0000060f), propget, helpstring("Returns/Sets whether this control can act as an OLE drop target."), helpcontext(0x00033692)] 
        OLEDropConstants#i OLEDropMode();
        [id(0x0000060f), propput, helpstring("Returns/Sets whether this control can act as an OLE drop target."), helpcontext(0x00033692)] 
        void OLEDropMode([in] OLEDropConstants#i rhs);
        [id(0xfffffdf8), propget, helpstring("Returns/sets whether or not controls, Forms or an MDIForm are painted at run time with 3-D effects."), helpcontext(0x0003354b)] 
        AppearanceConstants#i Appearance();
        [id(0xfffffdf8), propput, helpstring("Returns/sets whether or not controls, Forms or an MDIForm are painted at run time with 3-D effects."), helpcontext(0x0003354b)] 
        void Appearance([in] AppearanceConstants#i rhs);
        [id(0xfffffe08), propget, helpstring("Returns/sets the border style for an object."), helpcontext(0x0003354c)] 
        BorderStyleConstants#i BorderStyle();
        [id(0xfffffe08), propput, helpstring("Returns/sets the border style for an object."), helpcontext(0x0003354c)] 
        void BorderStyle([in] BorderStyleConstants#i rhs);
        [id(0xfffffdfe), propget, helpstring("Returns/sets a value that determines whether a form or control can respond to user-generated events."), helpcontext(0x0003354d)] 
        VARIANT_BOOL Enabled();
        [id(0xfffffdfe), propput, helpstring("Returns/sets a value that determines whether a form or control can respond to user-generated events."), helpcontext(0x0003354d)] 
        void Enabled([in] VARIANT_BOOL rhs);
        [id(0xfffffe00), propget, helpstring("Returns a Font object."), helpcontext(0x0003354e)] 
        IFontDisp#i* Font();
        [id(0xfffffe00), propput, helpstring("Returns a Font object."), helpcontext(0x0003354e)] 
        void Font([in] IFontDisp#i* rhs);
        [id(0xfffffe00), propputref, helpstring("Returns a Font object."), helpcontext(0x0003354e)] 
        void Font([in] IFontDisp#i* rhs);
        [id(0xfffffdfd), propget, helpstring("Returns a handle to a form or control."), helpcontext(0x0003354f)] 
        OLE_HANDLE#i hWnd();
        [id(0xfffffdfd), propput, helpstring("Returns a handle to a form or control."), helpcontext(0x0003354f)] 
        void hWnd([in] OLE_HANDLE#i rhs);
        [id(0x0000000e), helpstring("Returns a reference to the ListItem object or Node object located at the coordinates of x and y. Used with drag and drop operations."), helpcontext(0x00033550)] 
        INode#i* HitTest(
            [in] float x,
            [in] float y
        );
        [id(0x0000000f), helpstring("Returns the number of Node objects that fit in the internal area of a TreeView control."), helpcontext(0x00033551)] 
        long GetVisibleCount();
        [id(0x00000010), helpstring("Begins a label editing operation on a ListItem or Node object."), helpcontext(0x00033552)] 
        void StartLabelEdit();
        [id(0xfffffdda), helpstring("Forces a complete repaint of a form or control."), helpcontext(0x00033553)] 
        void Refresh();
        [id(0xfffffdd8), hidden] 
        void AboutBox();
        [id(0x00000610), helpstring("Starts an OLE drag/drop event with the given control as the source."), helpcontext(0x00033690)] 
        void OLEDrag();
        [id(0x00000011), propget, helpstring("Returns/sets a value which determines if the control displays a checkbox next to each item in the tree."), helpcontext(0x00033785)] 
        VARIANT_BOOL Checkboxes();
        [id(0x00000011), propput, helpstring("Returns/sets a value which determines if the control displays a checkbox next to each item in the tree."), helpcontext(0x00033785)] 
        void Checkboxes([in] VARIANT_BOOL rhs);
        [id(0x00000012), propget, helpstring("Returns/sets a value which determines if the entire row of the selected item is highlighted and clicking anywhere on an item's row causes it to be selected."), helpcontext(0x00033786)] 
        VARIANT_BOOL FullRowSelect();
        [id(0x00000012), propput, helpstring("Returns/sets a value which determines if the entire row of the selected item is highlighted and clicking anywhere on an item's row causes it to be selected."), helpcontext(0x00033786)] 
        void FullRowSelect([in] VARIANT_BOOL rhs);
        [id(0x00000013), propget, helpstring("Returns/sets a value which determines if items are highlighted as the mousepointer passes over them."), helpcontext(0x00033787)] 
        VARIANT_BOOL HotTracking();
        [id(0x00000013), propput, helpstring("Returns/sets a value which determines if items are highlighted as the mousepointer passes over them."), helpcontext(0x00033787)] 
        void HotTracking([in] VARIANT_BOOL rhs);
        [id(0x00000014), propget, helpstring("Returns/sets a value which determines if the TreeView displays scrollbars and allows scrolling (vertical and horizontal)."), helpcontext(0x00033788)] 
        VARIANT_BOOL Scroll();
        [id(0x00000014), propput, helpstring("Returns/sets a value which determines if the TreeView displays scrollbars and allows scrolling (vertical and horizontal)."), helpcontext(0x00033788)] 
        void Scroll([in] VARIANT_BOOL rhs);
        [id(0x00000015), propget, helpstring("Returns/sets a value which determines if selecting a new item in the tree expands that item and collapses the previously selected item."), helpcontext(0x00033789)] 
        VARIANT_BOOL SingleSel();
        [id(0x00000015), propput, helpstring("Returns/sets a value which determines if selecting a new item in the tree expands that item and collapses the previously selected item."), helpcontext(0x00033789)] 
        void SingleSel([in] VARIANT_BOOL rhs);
};

There is only this similar method

INode* HitTest(
            [in] float x,
            [in] float y
        );

Committed rev 11507 (4231b) should fix issues found in the review. getItem and add() methods were rewritten.

It seems that there is an issue in converting an unknown parameter given by ? mark in this code

hNode = chCtrlFrame:TreeView:NODES:ADD(?, 1, "Key0", "Label0").

hTNode.assign(hTreeView.unwrapTreeView().getNodes().unwrapTreeNodeCollection().add(new integer(1), new character("Key0"), new character("Label0")));

hTNode.assign(hTreeView.unwrapTreeView().getNodes().unwrapTreeNodeCollection().add(new integer(), new integer(1), new character("Key0"), new character("Label0")));

Sergey Ivanovskiy wrote:

Eugenie, I am not aware of this method. Could you help to find 4GL examples of GET-NODE-AT(X, Y)? I didn't find this method in
[...]

There is only this similar method
[...]

Something like this:

ctrlFrame:treecontrol:GetNodeAt(ipX, ipY)

Hynek, any comments/additions?

Sergey Ivanovskiy wrote:

Committed rev 11507 (4231b) should fix issues found in the review. getItem and add() methods were rewritten.

It seems that there is an issue in converting an unknown parameter given by ? mark in this code
[...]
that is converted
[...]
but the expected code must be
[...]
Investigating reasons.

How safe is to use 11507(4231b) for conversion? Or better to wait for your fix to issue above?

Eugenie Lyzenko wrote:

Sergey Ivanovskiy wrote:

Eugenie, I am not aware of this method. Could you help to find 4GL examples of GET-NODE-AT(X, Y)? I didn't find this method in
[...]

There is only this similar method
[...]

Something like this:
[...]
Taking integer pixel values and returning character node key if found. May be it is specific to(starting with) tree-list widget.

Hynek, any comments/additions?

GetNodeAt is only available on the <customer_specific_treelist> control (forwarded to GetNodeAt of the dxTreeList control).

Eugenie Lyzenko wrote:

Sergey Ivanovskiy wrote:

Committed rev 11507 (4231b) should fix issues found in the review. getItem and add() methods were rewritten.

It seems that there is an issue in converting an unknown parameter given by ? mark in this code
[...]
that is converted
[...]
but the expected code must be
[...]
Investigating reasons.

How safe is to use 11507(4231b) for conversion? Or better to wait for your fix to issue above?

Sorry, I was offline. The rev 11507 is safe. I didn't change anything that can break the project conversion for customers.

Sergey Ivanovskiy wrote:

Eugenie Lyzenko wrote:

Sergey Ivanovskiy wrote:

Committed rev 11507 (4231b) should fix issues found in the review. getItem and add() methods were rewritten.

It seems that there is an issue in converting an unknown parameter given by ? mark in this code
[...]
that is converted
[...]
but the expected code must be
[...]
Investigating reasons.

How safe is to use 11507(4231b) for conversion? Or better to wait for your fix to issue above?

Sorry, I was offline. The rev 11507 is safe. I didn't change anything that can break the project conversion for customers.

Not a problem, thanks. I have started new conversion for 11511.

Please correct if the following requirements are incorrect. It needs to implement GET-NODE-AT(X, Y) for the tree widget that must return the character key of the node under the mouse pointer or the comhandle that is under the mouse pointer. Now TreeViewGuiImpl already implement hitTest(x, y) that returns the node id under the mouse pointer. According to ThinClient.hitTest(int widgetId, int x, int y) it follows that it needs to implement hitTest for TreeListGuiImpl. Planning to check a working test case for hitTest.

Sergey Ivanovskiy wrote:

Please correct if the following requirements are incorrect. It needs to implement GET-NODE-AT(X, Y) for the tree widget that must return the character key of the node under the mouse pointer or the comhandle that is under the mouse pointer. Now TreeViewGuiImpl already implement hitTest(x, y) that returns the node id under the mouse pointer. According to ThinClient.hitTest(int widgetId, int x, int y) it follows that it needs to implement hitTest for TreeListGuiImpl. Planning to check a working test case for hitTest.

Correct, GET-NODE-AT should return character node key.

I rechecked these test cases:

./progressbar/progress_bar_test.p
./progressbar/progress_bar_test1.p
./progressbar/progress_bar_test2.p
./progressbar/progress_bar_test3.p
./progressbar/progress_bar_test4.p
./progressbar/progress_bar_test5.p
./progressbar/progress_bar_test6.p
./progressbar/progress_bar_test7.p

compile:
    [javac] Compiling 49 source files to /home/sbi/projects/testcases/uast/build/classes
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest1.java:489: error: no suitable method found for setMin(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMin(new integer(0));
    [javac]                                          ^
    [javac]     method ProgressBar.setMin(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMin(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest1.java:490: error: no suitable method found for setMax(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMax(new integer(100));
    [javac]                                          ^
    [javac]     method ProgressBar.setMax(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMax(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest1.java:491: error: no suitable method found for setValue(integer)
    [javac]          hProgressBar.unwrapProgressBar().setValue(new integer(58));
    [javac]                                          ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:86: error: no suitable method found for setValue(integer)
    [javac]             hProgressBar.unwrapProgressBar().setValue(new integer(0));
    [javac]                                             ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:97: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(plus(pbVal, 1));
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:108: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(minus(pbVal, 1));
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:119: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(pbVal);
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:301: error: no suitable method found for setMax(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMax(new integer(100));
    [javac]                                          ^
    [javac]     method ProgressBar.setMax(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMax(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:302: error: no suitable method found for setMin(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMin(new integer(0));
    [javac]                                          ^
    [javac]     method ProgressBar.setMin(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMin(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:303: error: no suitable method found for setValue(integer)
    [javac]          hProgressBar.unwrapProgressBar().setValue(new integer(30));
    [javac]                                          ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest2.java:319: error: no suitable method found for setValue(integer)
    [javac]             hProgressBar.unwrapProgressBar().setValue(iCount);
    [javac]                                             ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:88: error: no suitable method found for setValue(integer)
    [javac]             hProgressBar.unwrapProgressBar().setValue(new integer(0));
    [javac]                                             ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:99: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(plus(pbVal, 1));
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:110: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(minus(pbVal, 1));
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:121: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(pbVal);
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:328: error: no suitable method found for setMax(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMax(new integer(100));
    [javac]                                          ^
    [javac]     method ProgressBar.setMax(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMax(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:329: error: no suitable method found for setMin(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMin(new integer(0));
    [javac]                                          ^
    [javac]     method ProgressBar.setMin(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMin(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:330: error: no suitable method found for setValue(integer)
    [javac]          hProgressBar.unwrapProgressBar().setValue(new integer(30));
    [javac]                                          ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest3.java:344: error: no suitable method found for setValue(integer)
    [javac]             hProgressBar.unwrapProgressBar().setValue(iCount);
    [javac]                                             ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:92: error: no suitable method found for setValue(integer)
    [javac]             hProgressBar.unwrapProgressBar().setValue(new integer(0));
    [javac]                                             ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:103: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(plus(pbVal, 1));
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:114: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(minus(pbVal, 1));
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:125: error: no suitable method found for setValue(integer)
    [javac]                hProgressBar.unwrapProgressBar().setValue(pbVal);
    [javac]                                                ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:442: error: no suitable method found for setMax(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMax(new integer(100));
    [javac]                                          ^
    [javac]     method ProgressBar.setMax(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMax(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:443: error: no suitable method found for setMin(integer)
    [javac]          hProgressBar.unwrapProgressBar().setMin(new integer(0));
    [javac]                                          ^
    [javac]     method ProgressBar.setMin(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMin(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:444: error: no suitable method found for setValue(integer)
    [javac]          hProgressBar.unwrapProgressBar().setValue(new integer(30));
    [javac]                                          ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest4.java:459: error: no suitable method found for setValue(integer)
    [javac]             hProgressBar.unwrapProgressBar().setValue(iCount);
    [javac]                                             ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest5.java:46: error: no suitable method found for setMax(integer)
    [javac]          hProgBar.unwrapProgressBar().setMax(new integer(100));
    [javac]                                      ^
    [javac]     method ProgressBar.setMax(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMax(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest5.java:47: error: no suitable method found for setMin(integer)
    [javac]          hProgBar.unwrapProgressBar().setMin(new integer(0));
    [javac]                                      ^
    [javac]     method ProgressBar.setMin(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setMin(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest5.java:48: error: no suitable method found for setValue(integer)
    [javac]          hProgBar.unwrapProgressBar().setValue(new integer(30));
    [javac]                                      ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/progressbar/ProgressBarTest7.java:347: error: no suitable method found for setValue(integer)
    [javac]          hProgressBar.unwrapProgressBar().setValue(pValue_1);
    [javac]                                          ^
    [javac]     method ProgressBar.setValue(decimal) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to decimal)
    [javac]     method ProgressBar.setValue(double) is not applicable
    [javac]       (argument mismatch; integer cannot be converted to double)
    [javac] 31 errors

BUILD FAILED

Thus these tests were broken by recent changes in ocx conversion rules.

Sergey Ivanovskiy wrote:

I rechecked these test cases:

there are compilation errors now

This is after my most recent set of changes of ocx_conversion.rules. 4231b revision 11513.

Thanks, changing all these methods in ProgressBar to accept NumericType in rev 11454 solves this compilation issue.
I found another issue that if COM_PARAMETER is represented by an invocation expression, then the conversion is failed. I tested this code

hChildNode = chCtrlFrame:TreeView:NODES:Add(hNode:Index, 4, cKey, cText).

     [java] throwException(sprintf("Failed to resolve parameter type at index %s for class %s and method %s with %s expected parameters.Missing Java method declaration?", parent.getIndexPos(), invocClass.getName(), methodName, numParams))
     [java] ^  { Failed to resolve parameter type at index 2 for class com.goldencode.p2j.util.integer and method add with 4 expected parameters.Missing Java method declaration? [VAR_CHAR id <12884902782> 222:61] }
     [java] ---------------------------
     [java] ERROR:
     [java] com.goldencode.p2j.pattern.TreeWalkException: ERROR!  Active Rule:
     [java] -----------------------
     [java]       RULE REPORT      
     [java] -----------------------
     [java] Rule Type :   WALK
     [java] Source AST:  [ cKey ] BLOCK/STATEMENT/KW_ON/TRIGGER_BLOCK/BLOCK/ASSIGNMENT/ASSIGN/EXPRESSION/COM_INVOCATION/COM_METHOD/COM_PARAMETER/VAR_CHAR/ @222:61 {12884902782}
     [java] Copy AST  :  [ cKey ] BLOCK/STATEMENT/KW_ON/TRIGGER_BLOCK/BLOCK/ASSIGNMENT/ASSIGN/EXPRESSION/COM_INVOCATION/COM_METHOD/COM_PARAMETER/VAR_CHAR/ @222:61 {12884902782}
     [java] Condition :  throwException(sprintf("Failed to resolve parameter type at index %s for class %s and method %s with %s expected parameters.Missing Java method declaration?", parent.getIndexPos(), invocClass.getName(), methodName, numParams))
     [java] Loop      :  false
     [java] --- END RULE REPORT ---

                    <ast col="41" id="12884902769" line="222" text="Add" type="COM_METHOD">
                      <annotation datatype="java.lang.Long" key="oldtype" value="509"/>
                      <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                      <ast col="0" id="12884902771" line="0" text="" type="COM_PARAMETER">
                        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        <ast col="50" id="12884902772" line="222" text=":" type="COM_INVOCATION">
                          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                          <ast col="45" id="12884902773" line="222" text="hNode" type="VAR_COM_HANDLE">
                            <annotation datatype="java.lang.Long" key="oldtype" value="2891"/>
                            <annotation datatype="java.lang.Long" key="refid" value="12884902073"/>
                            <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                            <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                          </ast>
                          <ast col="51" id="12884902774" line="222" text="Index" type="COM_PROPERTY">
                            <annotation datatype="java.lang.Long" key="oldtype" value="689"/>
                            <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                          </ast>
                        </ast>
                      </ast>
                      <ast col="0" id="12884902777" line="0" text="" type="COM_PARAMETER">
                        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        <ast col="58" id="12884902778" line="222" text="4" type="NUM_LITERAL">
                          <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        </ast>
                      </ast>
                      <ast col="0" id="12884902781" line="0" text="" type="COM_PARAMETER">
                        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        <ast col="61" id="12884902782" line="222" text="cKey" type="VAR_CHAR">
                          <annotation datatype="java.lang.Long" key="oldtype" value="2891"/>
                          <annotation datatype="java.lang.Long" key="refid" value="12884902017"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        </ast>
                      </ast>
                      <ast col="0" id="12884902785" line="0" text="" type="COM_PARAMETER">
                        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        <ast col="67" id="12884902786" line="222" text="cText" type="VAR_CHAR">
                          <annotation datatype="java.lang.Long" key="oldtype" value="2891"/>
                          <annotation datatype="java.lang.Long" key="refid" value="12884902033"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
                        </ast>
                      </ast>
                    </ast>

Although the conversion of this code

hChildNode = chCtrlFrame:TreeView:NODES:Add(iNodeIndex, 4, cKey, cText).

                    <ast col="41" id="12884902780" line="224" text="Add" type="COM_METHOD">
                      <annotation datatype="java.lang.Long" key="oldtype" value="509"/>
                      <annotation datatype="java.lang.String" key="java-method" value="add"/>
                      <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                      <ast col="0" id="12884902782" line="0" text="" type="COM_PARAMETER">
                        <ast col="45" id="12884902783" line="224" text="iNodeIndex" type="VAR_INT">
                          <annotation datatype="java.lang.Long" key="oldtype" value="2891"/>
                          <annotation datatype="java.lang.Long" key="refid" value="12884901989"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Long" key="peerid" value="94489280955"/>
                        </ast>
                      </ast>
                      <ast col="0" id="12884902786" line="0" text="" type="COM_PARAMETER">
                        <ast col="57" id="12884902787" line="224" text="4" type="NUM_LITERAL">
                          <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Boolean" key="use64bit" value="false"/>
                          <annotation datatype="java.lang.Long" key="peerid" value="94489280957"/>
                        </ast>
                      </ast>
                      <ast col="0" id="12884902790" line="0" text="" type="COM_PARAMETER">
                        <ast col="60" id="12884902791" line="224" text="cKey" type="VAR_CHAR">
                          <annotation datatype="java.lang.Long" key="oldtype" value="2891"/>
                          <annotation datatype="java.lang.Long" key="refid" value="12884902017"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Long" key="peerid" value="94489280958"/>
                        </ast>
                      </ast>
                      <ast col="0" id="12884902794" line="0" text="" type="COM_PARAMETER">
                        <ast col="66" id="12884902795" line="224" text="cText" type="VAR_CHAR">
                          <annotation datatype="java.lang.Long" key="oldtype" value="2891"/>
                          <annotation datatype="java.lang.Long" key="refid" value="12884902033"/>
                          <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                          <annotation datatype="java.lang.Long" key="peerid" value="94489280959"/>
                        </ast>
                      </ast>
                    </ast>

I tested hitTest method on 4GL using this committed test case uast/treeview/test-treeview-sp6-1.w. chCtrlFrame:TreeView:HitTest(xPos, yPos) returns 0 if it is invoked from Click event handler:

PROCEDURE CtrlFrame.TreeView.Click : 
DEF VAR xPos AS DECIMAL.
DEF VAR yPos AS DECIMAL.
RUN getMousePoint(INPUT C-Win, OUTPUT xPos, OUTPUT yPos).
MESSAGE xPos " " yPos.

/* failed on 4GL */
/*hNode = chCtrlFrame:TreeView:HitTest(xPos, yPos).*/

/*succeeded on FWD */
hNode = chCtrlFrame:TreeView:HitTestFwd(xPos, yPos).

IF NOT VALID-HANDLE(hNode) THEN
DO:
MESSAGE "HitTest failed! " hNode.
RETURN.
END.

MESSAGE "HitTest succeeded! " hNode.

RUN printNode(Input hNode).

END PROCEDURE.

Sergey Ivanovskiy wrote:

Thanks, changing all these methods in ProgressBar to accept NumericType in rev 11454 solves this compilation issue.
I found another issue that if COM_PARAMETER is represented by an invocation expression, then the conversion is failed. I tested this code

Resolved in 4231b revision 11522.

Planning to convert COM-SELF into self() and unwrap the target type using provided java-class-name annotation so that this AST

              <ast col="0" id="12884903741" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281731"/>
                <ast col="39" id="12884903742" line="406" text=":" type="COM_INVOCATION">
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281732"/>
                  <ast col="33" id="12884903743" line="406" text=":" type="COM_INVOCATION">
                    <annotation datatype="java.lang.String" key="java-class-name" value="com.goldencode.p2j.ui.TreeFace"/>
                    <ast col="25" id="12884903744" line="406" text="COM-SELF" type="SYS_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="1148"/>
                      <annotation datatype="java.lang.Boolean" key="builtin" value="true"/>
                      <annotation datatype="java.lang.String" key="java-class-name" value="com.goldencode.p2j.ui.TreeFace"/>
                      <annotation datatype="java.lang.Boolean" key="hwrap" value="true"/>
                      <annotation datatype="java.lang.Long" key="peerid" value="94489281733"/>
                    </ast>
                    <ast col="34" id="12884903745" line="406" text="NODES" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2676"/>
                    </ast>
                  </ast>
                  <ast col="40" id="12884903746" line="406" text="COUNT" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="1176"/>
                    <annotation datatype="java.lang.String" key="java-method" value="COUNT"/>
                    <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  </ast>
                </ast>
              </ast>
            </ast>

Sergey Ivanovskiy wrote:

will be converted into this java expression self().unwrapTreeFace().getNodes().getCount(). It seems that for FWD the conversion of COM-SELF should be similar to SELF handle with provided java type information for unwrapping the target resource, correct?

Yes I think so. Since we know the target widget type for every handler unwrapping the correct type should be possible.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

will be converted into this java expression self().unwrapTreeFace().getNodes().getCount(). It seems that for FWD the conversion of COM-SELF should be similar to SELF handle with provided java type information for unwrapping the target resource, correct?

Yes I think so. Since we know the target widget type for every handler unwrapping the correct type should be possible.

At this moment I understand how to implement this idea with help of hint file

<ocx procedure-handler="CtrlFrame.TreeView.Click" com-handle="COM-SELF" target-widget="com.goldencode.p2j.ui.TreeFace" target-handle="COM-SELF"/>

<ocx com-handle="COM-SELF" target-widget="com.goldencode.p2j.ui.TreeFace" target-handle="COM-SELF"/>

              <ast col="0" id="12884903741" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281731"/>
                <ast col="39" id="12884903742" line="406" text=":" type="COM_INVOCATION">
                  <annotation datatype="java.lang.String" key="java-class-name" value="com.goldencode.p2j.util.integer"/>
                  <annotation datatype="java.lang.Boolean" key="hwrap" value="false"/>
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281732"/>
                  <ast col="33" id="12884903743" line="406" text=":" type="COM_INVOCATION">
                    <annotation datatype="java.lang.String" key="java-class-name" value="com.goldencode.p2j.ui.TreeNodeCollection"/>
                    <annotation datatype="java.lang.Boolean" key="hwrap" value="true"/>
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281734"/>
                    <ast col="25" id="12884903744" line="406" text="COM-SELF" type="SYS_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="1148"/>
                      <annotation datatype="java.lang.Boolean" key="builtin" value="true"/>
                      <annotation datatype="java.lang.String" key="java-class-name" value="com.goldencode.p2j.ui.TreeFace"/>
                      <annotation datatype="java.lang.Boolean" key="hwrap" value="true"/>
                      <annotation datatype="java.lang.Long" key="peerid" value="94489281735"/>
                    </ast>
                    <ast col="34" id="12884903745" line="406" text="NODES" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2676"/>
                      <annotation datatype="java.lang.String" key="java-method" value="getNodes"/>
                      <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                    </ast>
                  </ast>
                  <ast col="40" id="12884903746" line="406" text="COUNT" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="1176"/>
                    <annotation datatype="java.lang.String" key="java-method" value="getCount"/>
                    <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  </ast>
                </ast>
              </ast>

<ocx procedure-handler="CtrlFrame.TreeView.Click" com-handle="COM-SELF" target-widget="com.goldencode.p2j.ui.TreeFace" target-handle="COM-SELF"/>

Please review the committed revision 11526 (4231b) that added COM-SELF conversion with this hint file

<ocx procedure-handler="CtrlFrame.TreeView.Click" com-handle="COM-SELF" target-widget="com.goldencode.p2j.ui.TreeFace" target-handle="COM-SELF"/>

Sergey Ivanovskiy wrote:

Are there another ways to get target widget type when annotation rules are processed?

Yes, I think there is a simpler way, without a need to change the ext-hints file.

The UI handler procedure name is assembled from the control frame name, the root com property (which comes from wrx) and the actual method name defined on the COM event sink interface.

So if you take CtrlFrame.customer_tree1.OnChangeNode as an example, CtrlFrame is the control frame name, customer_tree1 is the root property and OnChangeNode is the event sink method name. Since COM-SELF is only valid in the event handler procedure body it is safe to only check for the control frame name to get the target type.

The target type in ocx_conversion.rules is stored in cf2wtype map. The key of the map is upper-cased control frame name.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Are there another ways to get target widget type when annotation rules are processed?

Yes, I think there is a simpler way, without a need to change the ext-hints file.

The UI handler procedure name is assembled from the control frame name, the root com property (which comes from wrx) and the actual method name defined on the COM event sink interface.

So if you take CtrlFrame.customer_tree1.OnChangeNode as an example, CtrlFrame is the control frame name, customer_tree1 is the root property and OnChangeNode is the event sink method name. Since COM-SELF is only valid in the event handler procedure body it is safe to only check for the control frame name to get the target type.

The target type in ocx_conversion.rules is stored in cf2wtype map. The key of the map is upper-cased control frame name.

Thank you. I see that now FWD doesn't support conversion of anywhere handlers. For an example, the following procedure name
PROCEDURE ANYWHERE.SpinUp is defined for all controls handled SpinUp event. Correct? I already committed rev 11526 (4231b). Should I change the way how the type was resolved to search with help of control frame?

The committed rev 11534(4231b) extracted the control frame name from the ActiveX event procedure name and then used it to find the widget type. If it is an ANYWHERE event procedure, then it tries to use this extended hint

<ocx procedure-handler="Anywhere.Click" com-handle="COM-SELF" target-widget="com.goldencode.p2j.ui.TreeFace" target-handle="COM-SELF"/> 

<ocx procedure-handler="Anywhere.Click" target-widget="com.goldencode.p2j.ui.TreeFace"/> 

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Are there another ways to get target widget type when annotation rules are processed?

Yes, I think there is a simpler way, without a need to change the ext-hints file.

The UI handler procedure name is assembled from the control frame name, the root com property (which comes from wrx) and the actual method name defined on the COM event sink interface.

So if you take CtrlFrame.customer_tree1.OnChangeNode as an example, CtrlFrame is the control frame name, customer_tree1 is the root property and OnChangeNode is the event sink method name. Since COM-SELF is only valid in the event handler procedure body it is safe to only check for the control frame name to get the target type.

The target type in ocx_conversion.rules is stored in cf2wtype map. The key of the map is upper-cased control frame name.

Thank you. I see that now FWD doesn't support conversion of anywhere handlers. For an example, the following procedure name
PROCEDURE ANYWHERE.SpinUp is defined for all controls handled SpinUp event. Correct?

Correct. I'm not even aware of such feature.

I already committed rev 11526 (4231b). Should I change the way how the type was resolved to search with help of control frame?

I would rather change this, I think the little extra work is worth so that the ext-hints file can stay stable.

The hitTest becomes working properly with hitTestFwd extension if these changes are applied

=== modified file 'src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java'
--- src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java    2020-02-18 22:42:08 +0000
+++ src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java    2020-05-08 16:58:12 +0000
@@ -591,8 +591,7 @@
     */      
    public Integer hitTest(int x, int y)
    {
-      NativePoint p = screenPhysicalLocation();
-      NativePoint point = new NativePoint(x - p.x, y - p.y);
+      NativePoint point = new NativePoint(x - tree.config.x, y - tree.config.y);
       TreeGuiImpl.TreeNode node = this.hitTest(point);
       return node != null && node.value != null ? node.value.nodeId : null;
    }   

PROCEDURE CtrlFrame.TreeView.Click : 
DEF VAR xPos AS DECIMAL.
DEF VAR yPos AS DECIMAL.
DEF VAR pnt AS INTEGER EXTENT 2.
RUN getMousePoint(INPUT C-Win, OUTPUT xPos, OUTPUT yPos).
MESSAGE xPos " " yPos.

DEF VAR hnd AS HANDLE.
hnd = chCtrlFrame:TreeView.

/*
pnt = hnd:GET-MOUSE-POSITION().

ASSIGN
xPos = pnt[1]
yPos = pnt[2].
*/

/* failed on 4GL */
/* hNode = chCtrlFrame:TreeView:HitTest(xPos, yPos).*/

/*succeeded on FWD */
hNode = chCtrlFrame:TreeView:HitTestFwd(xPos, yPos).

MESSAGE SELF:FRAME-X ", " SELF:FRAME-Y " " SELF:x ", " SELF:y " " SELF:WIDTH-PIXELS "x" SELF:HEIGHT-PIXELS.

MESSAGE "#(all nodes)=" COM-SELF:NODES:COUNT.

IF NOT VALID-HANDLE(hNode) THEN
DO:
MESSAGE "HitTest failed! " hNode.
RETURN.
END.

MESSAGE "HitTest succeeded! " hNode.

RUN printNode(Input hNode).

END PROCEDURE.

PROCEDURE ScreenToClient EXTERNAL "user32":
  DEFINE INPUT PARAMETER  hwnd AS LONG.
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

PROCEDURE GetCursorPos EXTERNAL "user32":
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

PROCEDURE getMousePoint :
  DEFINE INPUT  PARAMETER pcObject AS HANDLE     NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeXpos  AS DECIMAL    NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeYpos  AS DECIMAL    NO-UNDO.

  DEFINE VARIABLE mpLong  AS MEMPTR     NO-UNDO.

  SET-SIZE(mpLong) = 8.

  RUN getCursorPos(INPUT mpLong).

  RUN screenToClient(INPUT pcObject:HWND, INPUT mpLong).

  ASSIGN 
    pdeXpos = DECIMAL(GET-LONG(mpLong,1))
    pdeYpos = DECIMAL(GET-LONG(mpLong,5))
   .
END PROCEDURE.

PROCEDURE printNode :
DEFINE INPUT PARAMETER hNode AS COM-HANDLE.

MESSAGE "Key = '" hNode:Key "' Index = '" hNode:Index  "' Text = '" hNode:Text "'" " #(all nodes) = " chCtrlFrame:TreeView:NODES:COUNT.

END PROCEDURE.

Sergey Ivanovskiy wrote:

The hitTest becomes working properly with hitTestFwd extension if these changes are applied

Hynek, please review these changes.

You should use tree.physicalLocation() instead of tree.config.x[y]. The server-assigned config value may not always reflect the actual widget location.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

The hitTest becomes working properly with hitTestFwd extension if these changes are applied

Hynek, please review these changes.

You should use tree.physicalLocation() instead of tree.config.x[y]. The server-assigned config value may not always reflect the actual widget location.

Besides, the point coordinates passed to hitTest(NativePoint point) must be relative to the tree body origin - see in isNodeContentAtLocation int row = loc.y / nodeHeight;. If the passed in coordinates to hitTest(int x, int y) are already relative to windows origin, then the original version should be correct, no?

Hynek Cihlar wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

The hitTest becomes working properly with hitTestFwd extension if these changes are applied

Hynek, please review these changes.

You should use tree.physicalLocation() instead of tree.config.x[y]. The server-assigned config value may not always reflect the actual widget location.

Besides, the point coordinates passed to hitTest(NativePoint point) must be relative to the tree body origin - see in isNodeContentAtLocation int row = loc.y / nodeHeight;. If the passed in coordinates to hitTest(int x, int y) are already relative to windows origin, then the original version should be correct, no?

It seems that I already checked this case when the input coordinates x and y are relative to the window. On the 4GL FWD extension these coordinates should be
relative to the window origin.

DEF VAR hnd AS HANDLE.
hnd = chCtrlFrame:TreeView.

ASSIGN
xPos = hnd:OCX-MOUSE-X
yPos = hnd:OCX-MOUSE-Y.

I will check the original branch version again.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

The hitTest becomes working properly with hitTestFwd extension if these changes are applied

Hynek, please review these changes.

You should use tree.physicalLocation() instead of tree.config.x[y]. The server-assigned config value may not always reflect the actual widget location.

Besides, the point coordinates passed to hitTest(NativePoint point) must be relative to the tree body origin - see in isNodeContentAtLocation int row = loc.y / nodeHeight;. If the passed in coordinates to hitTest(int x, int y) are already relative to windows origin, then the original version should be correct, no?

It seems that I already checked this case when the input coordinates x and y are relative to the window. On the 4GL FWD extension these coordinates should be
relative to the window origin.
[...]

I will check the original branch version again.

Also check whether the passed in mouse coordinates are really relative to the window origin (including decorations) or its client area (excluding decorations).

The following 4GL code

DEF VAR xPos AS DECIMAL.
DEF VAR yPos AS DECIMAL.

RUN getMousePoint(INPUT C-Win, OUTPUT xPos, OUTPUT yPos).

MESSAGE xPos " " yPos.

PROCEDURE ScreenToClient EXTERNAL "user32":
  DEFINE INPUT PARAMETER  hwnd AS LONG.
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

PROCEDURE GetCursorPos EXTERNAL "user32":
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

PROCEDURE getMousePoint :
  DEFINE INPUT  PARAMETER pcObject AS HANDLE     NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeXpos  AS DECIMAL    NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeYpos  AS DECIMAL    NO-UNDO.

  DEFINE VARIABLE mpLong  AS MEMPTR     NO-UNDO.

  SET-SIZE(mpLong) = 8.

  RUN getCursorPos(INPUT mpLong).

  RUN screenToClient(INPUT pcObject:HWND, INPUT mpLong).

  ASSIGN 
    pdeXpos = DECIMAL(GET-LONG(mpLong,1))
    pdeYpos = DECIMAL(GET-LONG(mpLong,5))
   .
END PROCEDURE.

=== modified file 'src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java'
--- src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java    2020-02-18 22:42:08 +0000
+++ src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java    2020-05-08 16:58:12 +0000
@@ -591,8 +591,7 @@
     */      
    public Integer hitTest(int x, int y)
    {
-      NativePoint p = screenPhysicalLocation();
-      NativePoint point = new NativePoint(x - p.x, y - p.y);
+      NativePoint point = new NativePoint(x - tree.config.x, y - tree.config.y);
       TreeGuiImpl.TreeNode node = this.hitTest(point);
       return node != null && node.value != null ? node.value.nodeId : null;
    }   

If the input is relative to the window origin, then pnt = C-Win:GET-MOUSE-POSITION(). gets the mouse position for hitTest(x, y) method and the current implementation of hitTest in src/com/goldencode/p2j/ui/client/gui/TreeBodyGuiImpl.java can be left unchanged.

Hynek, please review the committed revision 11541(4231b) that implemented GET-NODE-AT. I used the following program to test these changes on FWD.

PROCEDURE CtrlFrame.TreeView.Click : 
DEF VAR xPos AS DECIMAL.
DEF VAR yPos AS DECIMAL.
DEF VAR pnt AS INTEGER EXTENT 2.

DEF VAR hnd AS HANDLE.

hnd = chCtrlFrame:TreeView.

DEF VAR nodeKey AS CHARACTER.

pnt = hnd:GET-MOUSE-POSITION().

MESSAGE "The mouse relative coordinates (" pnt[1] ", " pnt[2] ")".

Assign
xPos = pnt[1]
yPos = pnt[2].

nodeKey = chCtrlFrame:TreeView:GET-NODE-AT(xPos, yPos).

MESSAGE "GET-NODE-AT = " nodeKey.

END PROCEDURE.

It seems that MS tree view has inside a reference to another COM object via chCtrlFrame:TreeView:ImageList that holds indexed images so this method

chCtrlFrame:TreeView:NODES:ADD(1, 4, "Key", "Label", imageIndex, selectedImageIndex)

I found that ImageListWidget should implement these required features

Sergey Ivanovskiy wrote:

I found that ImageListWidget should implement these required features

Also look at TreeWidgetBase.createImage and TreeWidgetBase.setTreeNodeIcons.

None of the existing tests from testcases/uast/image-list can be run on 4GL.

Added uast/treeview/test-image-list-1.w to testcases project. In this 4GL example the ImageList is filled with help of LOAD-PICTURE that is not supported by FWD now but it is clear now how TreeView:Nodes:Add works with images.

Code review 4231b revision 11541.

ocx_conversion.rules, please revert the change. The addition of get-node-at -> getNodeAt is not needed. The OCX method identifier is GetNodeAt, which will be mapped to TreeNodeFace.getNodeAt OK.

TWB.getNodeAt, the method must not return null, instead return unknown character.

TC.findWidgetInContainerByCoordinates either returns widget id or a node id. But the caller has no way to interpret the meaning of the returned number. Also, why is this method needed when we already have TC.hitTest? Can this method be used directly?

Hynek Cihlar wrote:

Code review 4231b revision 11541.

ocx_conversion.rules, please revert the change. The addition of get-node-at -> getNodeAt is not needed. The OCX method identifier is GetNodeAt, which will be mapped to TreeNodeFace.getNodeAt OK.

TWB.getNodeAt, the method must not return null, instead return unknown character.

Did you test TreeNodeFace.getNodeAt without these changes in ocx_conversion.rules? I tested this code

DEF VAR nodeKey AS CHARACTER.
DEF VAR hnd AS HANDLE.
hnd = chCtrlFrame:TreeView.

pnt = hnd:GET-MOUSE-POSITION().

MESSAGE "4. " pnt[1] " " pnt[2].

Assign
xPos = pnt[1]
yPos = pnt[2].

nodeKey = chCtrlFrame:TreeView:GET-NODE-AT(xPos, yPos).

TC.findWidgetInContainerByCoordinates either returns widget id or a node id. But the caller has no way to interpret the meaning of the returned number.

Yes, nodes have identifiers but are not real widgets in this implementation.

Also, why is this method needed when we already have TC.hitTest? Can this method be used directly?

No, it is not possible to use it directly, because it takes coordinates related to the owner window. It seems that java doc explains why it is used. If you have another ideas please share them.

Rev 11542 (4231b) fixed getNodeAt to return unknown. But the following change leads to
the conversion becomes incorrect if

=== modified file 'rules/annotations/ocx_conversion.rules'
--- rules/annotations/ocx_conversion.rules    2020-05-11 19:40:31 +0000
+++ rules/annotations/ocx_conversion.rules    2020-05-12 19:20:56 +0000
@@ -314,7 +314,6 @@
          <action>map2.put("createcellicon"        , "createImage")</action>
          <action>map2.put("allowdragdropothertree", "DragDropOtherTree")</action>
          <action>map2.put("createsubnode2"        , "createSubNode")</action>
-         <action>map2.put("get-node-at"           , "getNodeAt")</action>
          <action>aliasMap.put("TREEVIEW"          , map2)</action>

          <!-- TREELIST -->

     [java] INFO: FWD OCX Extension: converted chCtrlFrame to hTreeView.
     [java] WARN: FWD OCX Extension: failed to resolve method GET-NODE-AT for the type com.goldencode.p2j.ui.TreeView
     [java] INFO: FWD OCX Extension: replaced DEFINE VARIABLE [hNode] with [hTNode]

nodeKey.assign(NoopCom.handle().call("GET-NODE-AT", ComParameter.input(xPos), ComParameter.input(yPos)));

Another argument for not using of hitTest directly on the server side is that on the server side we need to know the physical location of the tree widget relative to the owner window. It seems it can't be calculated on the server side directly, then it requires to call to the client side two times. The first one is to get coordinates and the next one is for hitTest itself. Correct?

Now the conversion of

PROCEDURE initialize-controls :

chCtrlFrame:ImageList:ImageWidth=32.
chCtrlFrame:ImageList:ImageHeight=32.

MESSAGE chCtrlFrame:ImageList:ImageWidth " " chCtrlFrame:ImageList:ImageHeight.

chImage = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand1.ico").
chCtrlFrame:ImageList:ListImages:ADD(1,"image1" , chImage).

chImage = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand2.ico").
chCtrlFrame:ImageList:ListImages:ADD(2,"image2" , chImage).

MESSAGE ":ImageList:ListImages:Count=" chCtrlFrame:ImageList:ListImages:Count.
chCtrlFrame-2:TreeView:ImageList=chCtrlFrame:ImageList.

chNode = chCtrlFrame-2:TreeView:NODES:Add().

chNode:Key = "key1".
chNode:Text = "Label1".
chNode:Image = 1.
chNode:SelectedImage = 2.

chCtrlFrame-2:TreeView:NODES:Add(1, 4, "key2", "Label2", 1, 2).

END PROCEDURE.

     [java] EXPRESSION EXECUTION ERROR:
     [java] ---------------------------
     [java] createProgressAst(#(int) widgetKwords.get(wType), wType, parRef)
     [java]                                       ^  { java.lang.NullPointerException }
     [java] ---------------------------
     [java] ERROR:
     [java] com.goldencode.p2j.pattern.TreeWalkException: ERROR!  Active Rule:
     [java] -----------------------
     [java]       RULE REPORT      
     [java] -----------------------
     [java] Rule Type :   WALK
     [java] Source AST:  [ CONTROL-FRAME ] BLOCK/STATEMENT/CREATE_WIDGET/KW_CNTRL_FR/ @144:8 {12884902302}
     [java] Copy AST  :  [ CONTROL-FRAME ] BLOCK/STATEMENT/CREATE_WIDGET/KW_CNTRL_FR/ @144:8 {12884902302}
     [java] Condition :  createProgressAst(#(int) widgetKwords.get(wType), wType, parRef)
     [java] Loop      :  false
     [java] --- END RULE REPORT ---
     [java] 
     [java] 
     [java] 
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1070)
     [java]     at com.goldencode.p2j.convert.TransformDriver.processTrees(TransformDriver.java:569)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.back(ConversionDriver.java:573)
     [java]     at com.goldencode.p2j.convert.TransformDriver.executeJob(TransformDriver.java:956)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1025)
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:39 [KW_CNTRL_FR id=12884902302]
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:275)
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:210)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.apply(PatternEngine.java:1633)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1531)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1479)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1034)
     [java]     ... 4 more
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:39
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:484)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:497)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:745)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:712)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:534)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:745)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:712)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:534)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:745)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:712)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:534)
     [java]     at com.goldencode.p2j.pattern.RuleContainer.apply(RuleContainer.java:585)
     [java]     at com.goldencode.p2j.pattern.RuleSet.apply(RuleSet.java:98)
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:262)
     [java]     ... 9 more
     [java] Caused by: java.lang.NullPointerException
     [java]     at com.goldencode.expr.CE6614.execute(Unknown Source)
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:391)
     [java]     ... 22 more

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Code review 4231b revision 11541.

ocx_conversion.rules, please revert the change. The addition of get-node-at -> getNodeAt is not needed. The OCX method identifier is GetNodeAt, which will be mapped to TreeNodeFace.getNodeAt OK.

TWB.getNodeAt, the method must not return null, instead return unknown character.

Did you test TreeNodeFace.getNodeAt without these changes in ocx_conversion.rules? I tested this code
[...]
and without these changes in ocx_conversion.rules the code above was converted incorrectly. Probably it is a new issue.

It fails, because you use incorrect method name: get-node-at instead of getNodeAt, which is the identifier declared on the OCX interface.

TC.findWidgetInContainerByCoordinates either returns widget id or a node id. But the caller has no way to interpret the meaning of the returned number.

Yes, nodes have identifiers but are not real widgets in this implementation.

As a caller there is no way to tell, whether you got a node id or a widget id of another unrelated widget which happens to be positioned on the supplied coordinates.

Also, why is this method needed when we already have TC.hitTest? Can this method be used directly?

No, it is not possible to use it directly, because it takes coordinates related to the owner window. It seems that java doc explains why it is used. If you have another ideas please share them.

Yes, the coordinate offset must happen on the client, because server has no information about the actual widget layout. To keep the API simple I would create new hitTest overload, perhaps also name it treeHitTest as the method is treeview and treelist specific, and introduce new enum param, that would identify the origin of the supplied coordinates - whether window top-left or window client area (aka window workspace) or possibly even screen 0,0 if needed.

Btw. the current implementation of findWidgetInContainerByCoordinates will fail with unexpected ClassCastException for TREELIST due to this cast if (container instanceof TreeViewGuiImpl). The same issue exists in TC.hitTest.

Sergey Ivanovskiy wrote:

Another argument for not using of hitTest directly on the server side is that on the server side we need to know the physical location of the tree widget relative to the owner window. It seems it can't be calculated on the server side directly, then it requires to call to the client side two times. The first one is to get coordinates and the next one is for hitTest itself. Correct?

I think it is enough to let hitTest now the meaning of the supplied coordinates. Whether it is relative to the window client area, window origin, or anything else. All the calculation then can be handled on the client. See my previous note.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Another argument for not using of hitTest directly on the server side is that on the server side we need to know the physical location of the tree widget relative to the owner window. It seems it can't be calculated on the server side directly, then it requires to call to the client side two times. The first one is to get coordinates and the next one is for hitTest itself. Correct?

I think it is enough to let hitTest now the meaning of the supplied coordinates. Whether it is relative to the window client area, window origin, or anything else. All the calculation then can be handled on the client. See my previous note.

Most of your arguments are subjective. I had alternatives to add parameters to hitTest or to add new treeHitTest or to use existing methods

Btw. the current implementation of findWidgetInContainerByCoordinates will fail with unexpected ClassCastException for TREELIST due to this cast if (container instanceof TreeViewGuiImpl). The same issue exists in TC.hitTest.

I didn't implement this method for TREELIST. Are there requirements to support the similar method for TREELIST?

Sergey Ivanovskiy wrote:

Now the conversion of
[...]
is failed with NPE
[...]
I prepared ext-hints for chCtrlFrame:ImageList and chCtrlFrame-2:TreeView and chNode. How can IImages be resolved in chCtrlFrame:ImageList:ListImages:ADD(1,"image1" , chImage).? How such expressions can be handled?

Currently ImageListWidget.getListImages returns comhandle with a technical COM object to comply with the expected COM interface. This mechanism was used before the most recent changes in ocx_conversion.rules, and is no longer needed since ocx_conversion.rules got the capability to unwrap handles to their expected types.

These are the changes to be done to make the above convert.
1. Make ListImages implement WrappedResource.
2. change ImageList.getListImages to return the type handle and the actual value new handle(new ListImages(this)).
3. annotate ImageList.getListImages with ResourceType(type = ListImages.class)
4. add new method @unwrapListImages to handle

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Another argument for not using of hitTest directly on the server side is that on the server side we need to know the physical location of the tree widget relative to the owner window. It seems it can't be calculated on the server side directly, then it requires to call to the client side two times. The first one is to get coordinates and the next one is for hitTest itself. Correct?

I think it is enough to let hitTest now the meaning of the supplied coordinates. Whether it is relative to the window client area, window origin, or anything else. All the calculation then can be handled on the client. See my previous note.

Most of your arguments are subjective. I had alternatives to add parameters to hitTest or to add new treeHitTest or to use existing methods

I'll be happy if you resolve at least the non-subjective ones :-).

Btw. the current implementation of findWidgetInContainerByCoordinates will fail with unexpected ClassCastException for TREELIST due to this cast if (container instanceof TreeViewGuiImpl). The same issue exists in TC.hitTest.

I didn't implement this method for TREELIST.

You did, by adding getNodeAt to TreeWidgetBase.

Are there requirements to support the similar method for TREELIST?

Yes.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Now the conversion of
[...]
is failed with NPE
[...]
I prepared ext-hints for chCtrlFrame:ImageList and chCtrlFrame-2:TreeView and chNode. How can IImages be resolved in chCtrlFrame:ImageList:ListImages:ADD(1,"image1" , chImage).? How such expressions can be handled?

Currently ImageListWidget.getListImages returns comhandle with a technical COM object to comply with the expected COM interface. This mechanism was used before the most recent changes in ocx_conversion.rules, and is no longer needed since ocx_conversion.rules got the capability to unwrap handles to their expected types.

These are the changes to be done to make the above convert.
1. Make ListImages implement WrappedResource.
2. change ImageList.getListImages to return the type handle and the actual value new handle(new ListImages(this)).
3. annotate ImageList.getListImages with ResourceType(type = ListImages.class)
4. add new method @unwrapListImages to handle

Thank you. Working on the found issues.

Hynek, could you add new testcases for TREELIST widget usages? I didn't find TREELIST examples in the testcases project repository.

Sergey Ivanovskiy wrote:

Hynek, could you add new testcases for TREELIST widget usages? I didn't find TREELIST examples in the testcases project repository.

Please see uast/demo/treelist.p.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek, could you add new testcases for TREELIST widget usages? I didn't find TREELIST examples in the testcases project repository.

Please see uast/demo/treelist.p.

Thank you. Testing it.

Hynek, could you clarify why TreeFace.getNodeAt has GET-NODE-AT legacy method annotation?

   /**
    * Returns key of the node at the specified location relative to the tree's top-left corner. Returns
    * unknown when no node is found.
    * This feature is currently not supported.
    *
    * @param   x
    *          The x pixel coordinate.
    * @param   y
    *          The y pixel coordinate.
    *
    * @return  key of the found key or unknown.
    */
   @LegacyMethod(name = "GET-NODE-AT")
   character getNodeAt(NumberType x, NumberType y);

Sergey Ivanovskiy wrote:

Hynek, could you clarify why TreeFace.getNodeAt has GET-NODE-AT legacy method annotation?
[...]

This is the legacy method identifier for the legacy pure 4GL interface. I.e. when you invoke the method through a handle which holds the TREEVIEW or TREELIST widget (no OCX involved).

I found an issue with ThinClient.getMousePosition(widgetId) for the Swing client related to AbstractWidget.displayPhysicalLocation().

   public int[] getMousePosition(int widgetId)
   {
      if (tk.isChui())
      {
         throw new UnsupportedOperationException();
      }

      GuiDriver gd = (GuiDriver) tk.getInstanceDriver();
      NativePoint p = gd.getMousePosition();
      System.out.println("mouse pointer " + p.x + ", " + p.y);
      Widget<?> w = null;

      if (widgetId != -1)
      {
         w = getWidget(widgetId);
      }

      if (w != null)
      {
         w = w.getLegacyWidget();
      }

      if (w != null)
      {
         NativePoint o = w.displayPhysicalLocation();
         System.out.println("widget displayPhysicalLocation " + o.x + ", " + o.y);
         p = p.minus(w.displayPhysicalLocation());
      }

      return new int[] {p.x, p.y};
   }

Hynek, could you help to explain why ThinClient.hitTest was in these setInvalidate-braces?

   public Integer hitTest(int widgetId, int x, int y)
   {
      Integer nodeId = null;
      Widget<?> widget = getWidget(widgetId);

      tk.setInvalidate(true);

      try
      {
         if (widget instanceof TreeGuiImpl)
         {
            nodeId = ((TreeGuiImpl) widget).hitTest(x, y);
         }
         cleanupQueue();
      }
      finally
      {
         tk.setInvalidate(widget, false, true);
      }

      return nodeId;
   }

Sergey Ivanovskiy wrote:

I found an issue with ThinClient.getMousePosition(widgetId) for the Swing client related to AbstractWidget.displayPhysicalLocation().

AbstractWidget.displayPhysicalLocation() returns incorrect coordinates if the owner window is located at the screen left top corner or moved back to the top edge or left top corner of the display screen. There are some bounds conditions that influence the returned result.

Do you mean when the window location is negative?

Sergey Ivanovskiy wrote:

Hynek, could you help to explain why ThinClient.hitTest was in these setInvalidate-braces?

No idea. I think this can be safely removed.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:
Now the conversion of
[...]
is failed with NPE
[...]
I prepared ext-hints for chCtrlFrame:ImageList and chCtrlFrame-2:TreeView and chNode. How can IImages be resolved in chCtrlFrame:ImageList:ListImages:ADD(1,"image1" , chImage).? How such expressions can be handled?

Currently ImageListWidget.getListImages returns comhandle with a technical COM object to comply with the expected COM interface. This mechanism was used before the most recent changes in ocx_conversion.rules, and is no longer needed since ocx_conversion.rules got the capability to unwrap handles to their expected types.

These are the changes to be done to make the above convert.
1. Make ListImages implement WrappedResource.

It seems that ListImages already implements WrappedResource via ComObject.

2. change ImageList.getListImages to return the type handle and the actual value new handle(new ListImages(this)).
3. annotate ImageList.getListImages with ResourceType(type = ListImages.class)
4. add new method @unwrapListImages to handle

Can these changes break the customer's code? I didn't know all usages of ImageList.

Sergey Ivanovskiy wrote:

Can these changes break the customer's code? I didn't know all usages of ImageList.

I hope not, but please do a grep on the customers' sources.

I found some usages:

hTreeView = controlFrameOfTree:COM-HANDLE.
hTree = hTreeView:CONTROLS:ITEM(1).
hImage = controlFrameOfImageList:COM-HANDLE.
hTree:ImageList  = hImage:CONTROLS:ITEM(1).
hImage:ImageList:ListImages:CLEAR().

Sergey Ivanovskiy wrote:

I found some usages:
[...]
and there are many actual usages of hImage:ImageList:ListImages:ADD.

So please test these cases with your changes. There should be no new issues. It is enough to extract these cases in simple uast test cases.

OK. I have a test case and planning to add more. Some open source 4GL code uses LOAD-PICTURE

DEFINE VARIABLE hPicture      AS COM-HANDLE.

    FILE-INFO:FILE-NAME = pcImageFile.

    IF FILE-INFO:FULL-PATHNAME <> ? THEN
    DO:
      hPicture = LOAD-PICTURE(FILE-INFO:FULL-PATHNAME).
    END.

    IF VALID-HANDLE(hPicture) THEN
    DO:
      ASSIGN chNewImage  = chImageList:ListImages:ADD(chImageList:ListImages:COUNT + 1, pcImageFile, hPicture).
             iImageIndex = chNewImage:INDEX.
      IF chImageList:ListImages:COUNT = 1 THEN DO:
          chTreeview:ImageList = chImageList.
      END.
    END.

Sergey Ivanovskiy wrote:

OK. I have a test case and planning to add more. Some open source 4GL code uses LOAD-PICTURE
[...]
LOAD-PICTURE should be supported by FWD, correct?

Currently it is not. If it is in a customer code base, than I suppose it should.

Please review rev 11550 (4231b) that implemented LOAD-PICTURE and ADD with complete set of parameters. I tested with ./treeview/test-image-list-1.w example from testcases project. This example shows that SelectedImage can't be mapped to ExpandNodeIcon. The behaviour of MS Tree is different on this example. The hand1.ico is displayed when this node doesn't have a focus and hand2.ico if it is focused and highlighted. The node can be collapsed and selected simultaneously. In FWD if the node is collapsed, then hand2.ico is displayed and if it is expanded, then hand1.ico is not dispalayed but the leaf node has hand1.ico in any of these two cases: selected or unselected.

Working on the issue from #4170-135.

LOAD-PICTURE should return OlePictureObject according to Progress Docs. It needs to add this new com object to FWD. I don't know what library exports this object. Now LOAD-PICTURE returns the wrapped instance of ListImage. MS Tree SelectImage of the tree node doesn't mean ExpandNodeIcon so it needs to add new getter and setter for this property to TreeNodeFace.

It seems that OlePictureObject is exported by olepro32.dll or oleaut32.dll, https://www.nirsoft.net/utils/dll_export_viewer.html logs these exported dll functions from olepro32.dll

==================================================
Function Name : DllCanUnloadNow
Address : 0x5f3126e0
Relative Address : 0x000126e0
Ordinal : 255 (0xff)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : DllGetClassObject
Address : 0x5f312700
Relative Address : 0x00012700
Ordinal : 256 (0x100)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : DllRegisterServer
Address : 0x5f312740
Relative Address : 0x00012740
Ordinal : 257 (0x101)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : DllUnregisterServer
Address : 0x5f312760
Relative Address : 0x00012760
Ordinal : 258 (0x102)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreateFontIndirect
Address : 0x5f312580
Relative Address : 0x00012580
Ordinal : 253 (0xfd)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreatePictureIndirect
Address : 0x5f3125a0
Relative Address : 0x000125a0
Ordinal : 252 (0xfc)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreatePropertyFrame
Address : 0x5f3125f0
Relative Address : 0x000125f0
Ordinal : 250 (0xfa)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreatePropertyFrameIndirect
Address : 0x5f312630
Relative Address : 0x00012630
Ordinal : 249 (0xf9)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleIconToCursor
Address : 0x5f312640
Relative Address : 0x00012640
Ordinal : 248 (0xf8)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleLoadPicture
Address : 0x5f3125c0
Relative Address : 0x000125c0
Ordinal : 251 (0xfb)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleTranslateColor
Address : 0x5f312560
Relative Address : 0x00012560
Ordinal : 254 (0xfe)
Filename : olepro32.dll
Full Path : C:\Progress\OE116_32\bin\system\olepro32.dll
Type : Exported Function ==================================================

----------------------------------------------------------------------------------------
from oleaut32.dll

==================================================
Function Name : OleCreateFontIndirect
Address : 0x7714a594
Relative Address : 0x0002a594
Ordinal : 420 (0x1a4)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreatePictureIndirect
Address : 0x7714a08a
Relative Address : 0x0002a08a
Ordinal : 419 (0x1a3)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreatePropertyFrame
Address : 0x77187777
Relative Address : 0x00067777
Ordinal : 417 (0x1a1)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleCreatePropertyFrameIndirect
Address : 0x77187729
Relative Address : 0x00067729
Ordinal : 416 (0x1a0)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleIconToCursor
Address : 0x77185167
Relative Address : 0x00065167
Ordinal : 415 (0x19f)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleLoadPicture
Address : 0x77185d80
Relative Address : 0x00065d80
Ordinal : 418 (0x1a2)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleLoadPictureEx
Address : 0x77185cc6
Relative Address : 0x00065cc6
Ordinal : 401 (0x191)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleLoadPictureFile
Address : 0x77187cdd
Relative Address : 0x00067cdd
Ordinal : 422 (0x1a6)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleLoadPictureFileEx
Address : 0x77187b85
Relative Address : 0x00067b85
Ordinal : 402 (0x192)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleLoadPicturePath
Address : 0x7718457c
Relative Address : 0x0006457c
Ordinal : 424 (0x1a8)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleSavePictureFile
Address : 0x77187c38
Relative Address : 0x00067c38
Ordinal : 423 (0x1a7)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

==================================================
Function Name : OleTranslateColor
Address : 0x7714bbb4
Relative Address : 0x0002bbb4
Ordinal : 421 (0x1a5)
Filename : oleaut32.dll
Full Path : C:\Progress\OE116_32\bin\system\oleaut32.dll
Type : Exported Function ==================================================

Sergey Ivanovskiy wrote:

Please review rev 11550 (4231b) that implemented LOAD-PICTURE and ADD with complete set of parameters. I tested with ./treeview/test-image-list-1.w example from testcases project. This example shows that SelectedImage can't be mapped to ExpandNodeIcon. The behaviour of MS Tree is different on this example. The hand1.ico is displayed when this node doesn't have a focus and hand2.ico if it is focused and highlighted. The node can be collapsed and selected simultaneously. In FWD if the node is collapsed, then hand2.ico is displayed and if it is expanded, then hand1.ico is not dispalayed but the leaf node has hand1.ico in any of these two cases: selected or unselected.

Code review 4231b 11550.

As you say, SelectedImage can't be mapped to ExpandNodeIcon. New property will be needed for this.

Regarding the following code block:

         <rule>ftype == prog.kw_load_pic
            <action>methodText = "com.goldencode.p2j.ui.ocx.ListImage.newInstance"</action>
            <action>methodType = java.static_method_call</action>
         </rule>

The following code

      if (parent != null)
      {
         return parent.valid();
      }

      return false;

can be simplified to

         return parent != null && parent.valid();

When one of the images (ListImage) is destroyed or removed the image index will have to shift. I wonder if you should cache the index in the ListImage instances, this will complicate things.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Please review rev 11550 (4231b) that implemented LOAD-PICTURE and ADD with complete set of parameters. I tested with ./treeview/test-image-list-1.w example from testcases project. This example shows that SelectedImage can't be mapped to ExpandNodeIcon. The behaviour of MS Tree is different on this example. The hand1.ico is displayed when this node doesn't have a focus and hand2.ico if it is focused and highlighted. The node can be collapsed and selected simultaneously. In FWD if the node is collapsed, then hand2.ico is displayed and if it is expanded, then hand1.ico is not dispalayed but the leaf node has hand1.ico in any of these two cases: selected or unselected.

Code review 4231b 11550.

As you say, SelectedImage can't be mapped to ExpandNodeIcon. New property will be needed for this.

Regarding the following code block:
[...]
This is not a serious issue, but I think it would help the code clarity to introduce a new method loadPicture on a helper class. ListImage.newInstance doesn't mentally map to LOAD-PICTURE and thus would be more difficult to find when working with the code in the future.

The following code
[...]

can be simplified to
[...]

When one of the images (ListImage) is destroyed or removed the image index will have to shift. I wonder if you should cache the index in the ListImage instances, this will complicate things.

There is 4GL code #4170-132 that is used in customer's projects. In this code example INDEX is cached so it needs to implement all these fields. The current FWD implementation supposes that KEY is an image path, but on the native 4GL system KEY of an image is any character identifier. ListImages didn't implement remove() of ListImage and getItem() by INDEX and KEY and it should be fixed.
Planning to use new unity OlePictureObject for LOAD-PICTURE result that actually holds the physical path to the binary image file.

Sergey Ivanovskiy wrote:

When one of the images (ListImage) is destroyed or removed the image index will have to shift. I wonder if you should cache the index in the ListImage instances, this will complicate things.

There is 4GL code #4170-132 that is used in customer's projects. In this code example INDEX is cached so it needs to implement all these fields. The current FWD implementation supposes that KEY is an image path, but on the native 4GL system KEY of an image is any character identifier. ListImages didn't implement remove() of ListImage and getItem() by INDEX and KEY and it should be fixed.
Planning to use new unity OlePictureObject for LOAD-PICTURE result that actually holds the physical path to the binary image file.

Sounds good. If image removal is not implemented yet, then the index caching is OK.

Hynek, I didn't understand what you meant by "If image removal is not implemented yet, then the index caching is OK.". Please explain more thoroughly.

Sergey Ivanovskiy wrote:

Hynek, I didn't understand what you meant by "If image removal is not implemented yet, then the index caching is OK.". Please explain more thoroughly.

The added ListImage.index will become invalid when one of the previous images is removed. But currently there is no way an image can be removed, unless I missed something.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek, I didn't understand what you meant by "If image removal is not implemented yet, then the index caching is OK.". Please explain more thoroughly.

The added ListImage.index will become invalid when one of the previous images is removed. But currently there is no way an image can be removed, unless I missed something.

Thanks, understand now. I am most hard of thinking :)

In this example the same OlePictureObject is added to image list widget with different keys and the result images have different keys and indices.

chExpPicture = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand2.ico").

chExpImage = chCtrlFrame:ImageList:ListImages:ADD(2, "image2", chExpPicture).

chExpImage2 = chCtrlFrame:ImageList:ListImages:ADD(3, "image3", chExpPicture).

message "chExpImage:index = " chExpImage:index " chExpImage:key ="  chExpImage:key " chExpImage2:index = " chExpImage2:index " chExpImage2:key ="  chExpImage2:key.

The following code doesn't warn about the same index and the list images count becomes 3.

chPicture = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand1.ico").

chImage = chCtrlFrame:ImageList:ListImages:ADD(1, "image1", chPicture).

chExpPicture = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand2.ico").

chExpImage = chCtrlFrame:ImageList:ListImages:ADD(2, "image2", chExpPicture).

chExpImage2 = chCtrlFrame:ImageList:ListImages:ADD(1, "image3", chExpPicture).

message "chExpImage:index = " chExpImage:index " chExpImage:key ="  chExpImage:key " chExpImage2:index = " chExpImage2:index " chExpImage2:key ="  chExpImage2:key view-as alert-box.

MESSAGE ":ImageList:ListImages:Count=" chCtrlFrame:ImageList:ListImages:Count.

chPicture = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand1.ico").

chImage = chCtrlFrame:ImageList:ListImages:ADD(1, "image1", chPicture).

chExpPicture = LOAD-PICTURE("Y:\\4gl\\testcases\\treeview\\Hand2.ico").

chExpImage = chCtrlFrame:ImageList:ListImages:ADD(2, "image2", chExpPicture).

chExpImage2 = chCtrlFrame:ImageList:ListImages:ADD(3, "image1", chExpPicture).

I delayed to commit my changes until the following issue was resolved. The same code

   @LegacySignature(type = Type.PROCEDURE, name = "initialize-controls")
   public void initializeControls()
   {
      internalProcedure(new Block((Body) () -> 
      {
         hImageList.unwrapImageList().setImageWidth(new integer(32));
         hImageList.unwrapImageList().setImageHeight(new integer(32));
         message(new Object[]
         {
            hImageList.unwrapImageList().getImageWidth(),
            " ",
            hImageList.unwrapImageList().getImageHeight()
         });
         chPicture.assign(com.goldencode.p2j.ui.ocx.PictureObject.loadPicture("Y:\\4gl\\testcases\\treeview\\Hand1.ico"));
         chImage.assign(hImageList.unwrapImageList().getListImages().unwrapListImages().add(new integer(1), new character("image1"), chPicture));
         chExpPicture.assign(com.goldencode.p2j.ui.ocx.PictureObject.loadPicture("Y:\\4gl\\testcases\\treeview\\Hand2.ico"));
         chExpImage.assign(hImageList.unwrapImageList().getListImages().unwrapListImages().add(new integer(2), new character("image2"), chExpPicture));
         message(new Object[]
         {
            ":ImageList:ListImages:Count=",
            hImageList.unwrapImageList().getListImages().unwrapListImages().getCount()
         });
         hTreeView.unwrapTreeView().setImageList(hImageList.unwrapImageList());
         hTNode.assign(hTreeView.unwrapTreeView().getNodes().unwrapTreeNodeCollection().add());
         hTNode.unwrapTreeNodeFace().setNodeKey(new character("key1"));
         hTNode.unwrapTreeNodeFace().setNodeText(new character("Label1"));
         hTNode.unwrapTreeNodeFace().setNodeIcon(chImage.getProperty("Index")); // <-----------------------------(*)
         hTNode.unwrapTreeNodeFace().setSelectedNodeIcon(chExpImage.getProperty("Index"));
         hTreeView.unwrapTreeView().getNodes().unwrapTreeNodeCollection().add(new integer(1), new integer(4), new character("key2"), new character("Label2"), ((NumberType) chImage.getProperty("Index")), ((NumberType) chExpImage.getProperty("Index")));
      }));
   }
}

Today I found this helpful article about MS Tree but some information was already discovered manually using 4GL code https://www.levelextreme.com/Home/ShowHeader?Activator=23&ID=39020

Sergey Ivanovskiy wrote:

These changes with treeview/test-image-list-1.w reproduced #4170-147.

Code review 4170_15.patch.

ListImage.getKey and LI.getIndex should handle the case when getListImages returns null.
LI.getTag must never return null.
Do not declare legacy methods with raw Java types, you may get in OCX conversion issues sooner or later. See ListImages.getItem.
If LI.findKey and LI.findIndex can be made package private, then please do so.
PO.getActivexName javadoc says it should return "OlePictureObject", but the method returns the simple class name.
PO.imageId should be initialized to -1, so that it can be used in the validity check. I can see it gets assigned in the ctor, but I would still initialize it - in case of an exception in the ctor or later code changes.
The added well known object collection error codes should not be placed in ErrorManager. At this level EM should not be bound to any particular component subjected to error handling.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

These changes with treeview/test-image-list-1.w reproduced #4170-147.

Code review 4170_15.patch.

ListImage.getKey and LI.getIndex should handle the case when getListImages returns null.
LI.getTag must never return null.
Do not declare legacy methods with raw Java types, you may get in OCX conversion issues sooner or later. See ListImages.getItem.
If LI.findKey and LI.findIndex can be made package private, then please do so.
PO.getActivexName javadoc says it should return "OlePictureObject", but the method returns the simple class name.
PO.imageId should be initialized to -1, so that it can be used in the validity check. I can see it gets assigned in the ctor, but I would still initialize it - in case of an exception in the ctor or later code changes.
The added well known object collection error codes should not be placed in ErrorManager. At this level EM should not be bound to any particular component subjected to error handling.

Thank you for this review. Could you advise the proper place for ErrorManager.ErrorReason enumeration?

The issue from #4170-167 was due to comhandle.resourceId usages in comhandle.compareTo that is used from object equals method when doing a search via linked list.

   public int compareTo(Object obj)
   {
      // handle is also a BDT type
      if (obj instanceof comhandle)
      {
         comhandle that = (comhandle) obj;

         // two unknown values are equals
         if (isUnknown() || that.isUnknown())
         {
            // an unknown handle is greater than a known handle
            return Boolean.compare(isUnknown(), that.isUnknown());
         }

         return (comhandle.resourceId(this.value) == comhandle.resourceId(that.value)) ? 0 : 1;
      }

      return -1;
   }

   /**
    * Factory method.
    * 
    * @param   parent
    *          Parent widget.
    *          
    * @return a new instance wrapped in a comhandle.
    */
   public static comhandle newInstance(GenericWidget parent, comhandle picture)
   {
      ListImage comObject = new ListImage(parent, picture);
      comhandle selfReference = new comhandle(comObject);
      comObject.self = selfReference;
      comhandle.resourceId(comObject); // workaround

      return selfReference;
   }

Working on the review fixes and javadocs, planning to commit ASAP.

Sergey Ivanovskiy wrote:

Thank you for this review. Could you advise the proper place for ErrorManager.ErrorReason enumeration?

Perhaps in the collection resource interface.

Sergey Ivanovskiy wrote:

The issue from #4170-167 was due to comhandle.resourceId usages in comhandle.compareTo that is used from object equals method when doing a search via linked list.
resourceId registers a resource. The following changes fixed this issue. It is a workaround.

Unless I miss something, you added comhandle.resourceId(comObject); so that comObject is assigned a resource id, correct? In the compareTo method resourceId is also called for the the object, so it shoold get assigned during the comparison. So why the added call to resourceId?

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

The issue from #4170-167 was due to comhandle.resourceId usages in comhandle.compareTo that is used from object equals method when doing a search via linked list.
resourceId registers a resource. The following changes fixed this issue. It is a workaround.

Unless I miss something, you added comhandle.resourceId(comObject); so that comObject is assigned a resource id, correct? In the compareTo method resourceId is also called for the the object, so it shoold get assigned during the comparison. So why the added call to resourceId?

I am not sure but this code

public int compareTo(Object obj)
   {
      // handle is also a BDT type
      if (obj instanceof comhandle)
      {
         comhandle that = (comhandle) obj;

         // two unknown values are equals
         if (isUnknown() || that.isUnknown())
         {
            // an unknown handle is greater than a known handle
            return Boolean.compare(isUnknown(), that.isUnknown());
         }

         return (comhandle.resourceId(this.value) == comhandle.resourceId(that.value)) ? 0 : 1;
      }

      return -1;
   }

(comhandle.resourceId(this.value) == comhandle.resourceId(that.value))

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

In node TreeNodeFace (and the implementing class), only keep setSelectedNodeIcon(NumberType imageId) and remove setSelectedNodeIcon(BaseDataType imageId). The passed in argument value must always be a number AFAIK.

TreeNodeResource.setNodeIcon, the expand node icon will retain the old image when the method is called twice. See the expression expandIconId == -1.

ListImage.setPicture, I think you should deref the image being replaced with the new image.

ListIMages.findKeyValueEntry, ListImages.images, ListImages.keysToImages, consider storing the actual values held by comhandles in these collections. In case where you have multiple handles pointing at the same value, you may save some space.

Otherwise the changes are good.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

The issue from #4170-167 was due to comhandle.resourceId usages in comhandle.compareTo that is used from object equals method when doing a search via linked list.
resourceId registers a resource. The following changes fixed this issue. It is a workaround.

Unless I miss something, you added comhandle.resourceId(comObject); so that comObject is assigned a resource id, correct? In the compareTo method resourceId is also called for the the object, so it shoold get assigned during the comparison. So why the added call to resourceId?

I am not sure but this code
can be incorrect if

is evaluated in parallel. It can return false even this.value === that.value.

If we can expect this method to be invoked from multiple threads, then it should be made thread safe I think. I'm not saying your change is incorrect, but we may encounter other similar cases.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

In node TreeNodeFace (and the implementing class), only keep setSelectedNodeIcon(NumberType imageId) and remove setSelectedNodeIcon(BaseDataType imageId). The passed in argument value must always be a number AFAIK.

I added this method because the following converted code required this method signature

hTNode.unwrapTreeNodeFace().setSelectedNodeIcon(chExpImage.getProperty("Index"));

TreeNodeResource.setNodeIcon, the expand node icon will retain the old image when the method is called twice. See the expression expandIconId == -1.

Thanks, I missed it.

ListImage.setPicture, I think you should deref the image being replaced with the new image.

Yes, I also found this issue but didn't commit changes yet.

ListIMages.findKeyValueEntry, ListImages.images, ListImages.keysToImages, consider storing the actual values held by comhandles in these collections. In case where you have multiple handles pointing at the same value, you may save some space.

It seems that there shouldn't be two instances pointing the same ListImage value because the image was created at ListImages.add. The same PictureObject object wrapped in comhandle can belong to different ListImage objects. If we would store ListImage, then it was only required the additional unwrap comhandle code but the same memory should be consumed. Please correct if there are gaps in these statements.

Guys,

Starting with 11569 the 4231b has still a conversion regression for customer's application. Is somebody working on the fix?

Eugenie Lyzenko wrote:

Guys,

Starting with 11569 the 4231b has still a conversion regression for customer's application. Is somebody working on the fix?

Is the regression related to OO? If so, Constantin made some updates in 4231b-11576 that fixed something that started in rev 11546.

Roger Borrello wrote:

Eugenie Lyzenko wrote:

Guys,

Starting with 11569 the 4231b has still a conversion regression for customer's application. Is somebody working on the fix?

Is the regression related to OO?

May be, I have not debugged it.

If so, Constantin made some updates in 4231b-11576 that fixed something that started in rev 11546.

11576 does not fix it.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

In node TreeNodeFace (and the implementing class), only keep setSelectedNodeIcon(NumberType imageId) and remove setSelectedNodeIcon(BaseDataType imageId). The passed in argument value must always be a number AFAIK.

I added this method because the following converted code required this method signature

The conversion script should cast the expression to NumberType if you remove the BDT overload.

ListIMages.findKeyValueEntry, ListImages.images, ListImages.keysToImages, consider storing the actual values held by comhandles in these collections. In case where you have multiple handles pointing at the same value, you may save some space.

It seems that there shouldn't be two instances pointing the same ListImage value because the image was created at ListImages.add. The same PictureObject object wrapped in comhandle can belong to different ListImage objects. If we would store ListImage, then it was only required the additional unwrap comhandle code but the same memory should be consumed. Please correct if there are gaps in these statements.

If there can be only one comhandle instance, then it is OK.

Eugenie Lyzenko wrote:

If so, Constantin made some updates in 4231b-11576 that fixed something that started in rev 11546.

11576 does not fix it.

Anyway, it does not belong to this Redmine issue.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

In node TreeNodeFace (and the implementing class), only keep setSelectedNodeIcon(NumberType imageId) and remove setSelectedNodeIcon(BaseDataType imageId). The passed in argument value must always be a number AFAIK.

I added this method because the following converted code required this method signature

The conversion script should cast the expression to NumberType if you remove the BDT overload.

It seems that it doesn't work as expected. If there doesn't exist the method with this signature, then the converted code doesn't cast to NumberType and the converted code is not compiled.

compile:
    [javac] Compiling 26 source files to /home/sbi/projects/testcases/uast/build/classes
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/treeview/TestImageList1.java:350: error: incompatible types: BaseDataType cannot be converted to NumberType
    [javac]          hTNode.unwrapTreeNodeFace().setSelectedNodeIcon(chExpImage.getProperty("Index"));

I see that there are setNodeIcon and setExpandNodeIcon with the same parameter of BaseDataType. The original code is

chNode:Image = chImage:Index.
chNode:SelectedImage = chExpImage:Index.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

In node TreeNodeFace (and the implementing class), only keep setSelectedNodeIcon(NumberType imageId) and remove setSelectedNodeIcon(BaseDataType imageId). The passed in argument value must always be a number AFAIK.

I added this method because the following converted code required this method signature

The conversion script should cast the expression to NumberType if you remove the BDT overload.

It seems that it doesn't work as expected. If there doesn't exist the method with this signature, then the converted code doesn't cast to NumberType and the converted code is not compiled.
[...]
What scripts are responsible for BaseDataType to NumberType conversion?

ocx_conversion.rules and ocx_conversion_post.rules. I will check that, for now use BaseDataType for the parameter. Eventually it must be changed to NumberType together with the other image methods.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

TreeNodeResource.setNodeIcon, the expand node icon will retain the old image when the method is called twice. See the expression expandIconId == -1.

I don't know all requirements, the first assumption was that if expand icon was not set directly, then it will be assigned with icon that represents the icon for the terminal node. Thus the requirement for compatibility with the previous version and MS Tree requires to consider the priority of assignments. If the expand icon has been set directly, then it can't be changed by setNodeIcon method; and if it is not set directly via setExpandNodeIcon, then setNodeIcon can change it accordingly.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Committed revision 11575 and 11574 (4231b) fixed the found issues.

Code review 4231b revisions 11575 and 11574.

TreeNodeResource.setNodeIcon, the expand node icon will retain the old image when the method is called twice. See the expression expandIconId == -1.

I don't know all requirements, the first assumption was that if expand icon was not set directly, then it will be assigned with icon that represents the icon for the terminal node. Thus the requirement for compatibility with the previous version and MS Tree requires to consider the priority of assignments. If the expand icon has been set directly, then it can't be changed by setNodeIcon method; and if it is not set directly via setExpandNodeIcon, then setNodeIcon can change it accordingly.

Another way is to create new class that represents MS Tree compatible tree and to preserve the previous version of tree. What ways should we follow to implement this logic in the same class or to create new version of the tree?

Sergey Ivanovskiy wrote:

I don't know all requirements, the first assumption was that if expand icon was not set directly, then it will be assigned with icon that represents the icon for the terminal node. Thus the requirement for compatibility with the previous version and MS Tree requires to consider the priority of assignments. If the expand icon has been set directly, then it can't be changed by setNodeIcon method; and if it is not set directly via setExpandNodeIcon, then setNodeIcon can change it accordingly.

We must provide compatibility with MS Treeview and dxTreelist. Any legacy code will either use one or the other, so we don't have any strict requirement for the runtime behavior if you use both of these interfaces in FWD at the same time.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

I don't know all requirements, the first assumption was that if expand icon was not set directly, then it will be assigned with icon that represents the icon for the terminal node. Thus the requirement for compatibility with the previous version and MS Tree requires to consider the priority of assignments. If the expand icon has been set directly, then it can't be changed by setNodeIcon method; and if it is not set directly via setExpandNodeIcon, then setNodeIcon can change it accordingly.

We must provide compatibility with MS Treeview and dxTreelist. Any legacy code will either use one or the other, so we don't have any strict requirement for the runtime behavior if you use both of these interfaces in FWD at the same time.

Please review 11580, 11579 (4231b) that should fix the review issues.

Is there any remaining work for this task?

This property

hTree:DropHighlight = chNode.

Sergey Ivanovskiy wrote:

It seems only DropHighLight is not implemented but it can be implemented as SELECTED-NODE until new logic will not be discovered.

Please finish this.

Greg Shah wrote:

Sergey Ivanovskiy wrote:

It seems only DropHighLight is not implemented but it can be implemented as SELECTED-NODE until new logic will not be discovered.

Please finish this.

I committed rev 11581 (4231b) that implemented DropHighLight as SELECTED-NODE.

Is this task ready for test status/100% completed?

Sergey Ivanovskiy wrote:

Greg Shah wrote:

Sergey Ivanovskiy wrote:

It seems only DropHighLight is not implemented but it can be implemented as SELECTED-NODE until new logic will not be discovered.

Please finish this.

I committed rev 11581 (4231b) that implemented DropHighLight as SELECTED-NODE.

DropHighlight is used to visually indicate the target node of the drag & drop operation, correct? If so its semantics will be slightly different from the selected node.

• Compare any visual differences of the selected node (set by SELECTED-NODE) and "highlighted" node set by DropHighLight.
• What happens when you set the highlight to first or last node, will the tree scroll the nodes? It should.
• What happens when you unset the highlight (by setting the default value)? Will the focus go back to the selected node?

MS Tree View doesn't expose SELECTED-NODE, but its node exposes SELECTED attribute. SELECTED-NODE is a FWD extension. There are differences between

chNode:SELECTED = true.

chCtrlFrame-2:TreeView:DropHighlight = chNode.

Sergey Ivanovskiy wrote:

MS Tree View doesn't expose SELECTED-NODE, but its node exposes SELECTED attribute. SELECTED-NODE is a FWD extension.

True, but this doesn't matter. I meant to compare the "selected node" and "drop highlight" features. The "drop highlight" is there to visualize the currently dragged over node during drag & drop operation. Visually this may be close or the same as "selected node". From the user's perspective if you drag over the first or last node, the tree should scroll so that you can drop the dragging node on the nodes out of the view port. I think this is also the responsibility of DropHighlight, when you set it to first or last node, the tree should scroll.

It doesn't look functional. It looks like a persistent selection. Planning to prepare 4GL testcases with drag and drop functionality applied to the tree.

What do you mean by "it doesn't look functional"?

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

MS Tree View doesn't expose SELECTED-NODE, but its node exposes SELECTED attribute. SELECTED-NODE is a FWD extension.

True, but this doesn't matter. I meant to compare the "selected node" and "drop highlight" features. The "drop highlight" is there to visualize the currently dragged over node during drag & drop operation. Visually this may be close or the same as "selected node". From the user's perspective if you drag over the first or last node, the tree should scroll so that you can drop the dragging node on the nodes out of the view port. I think this is also the responsibility of DropHighlight, when you set it to first or last node, the tree should scroll.

Actually SELECTED attribute should be added to TreeNodeFace. I missed to add it.

It doesn't look functional. It looks like a persistent selection. Planning to prepare 4GL testcases with drag and drop functionality applied to the tree.

What do you mean by "it doesn't look functional"?

1. SELECTED expands the hidden node, but DropHighlight doesn't.
2. SELECTED can be changed by the mouse, but DropHighlight can't be set via mouse selection because it attracts the focus persistently.
3. Setting OleDrapMode and OleDropMode to the manual mode have no effect on DropHighlight node. It behaves the same for this highlighted node and for the different node.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

MS Tree View doesn't expose SELECTED-NODE, but its node exposes SELECTED attribute. SELECTED-NODE is a FWD extension.

True, but this doesn't matter. I meant to compare the "selected node" and "drop highlight" features. The "drop highlight" is there to visualize the currently dragged over node during drag & drop operation. Visually this may be close or the same as "selected node". From the user's perspective if you drag over the first or last node, the tree should scroll so that you can drop the dragging node on the nodes out of the view port. I think this is also the responsibility of DropHighlight, when you set it to first or last node, the tree should scroll.

Actually SELECTED attribute should be added to TreeNodeFace. I missed to add it.

It doesn't look functional. It looks like a persistent selection. Planning to prepare 4GL testcases with drag and drop functionality applied to the tree.

What do you mean by "it doesn't look functional"?

I meant that I have no useful examples to prove that it works as you expected. If you have some validity tests, then please commit them into testcases project.

Unfortunately I don't have test cases for this. Please test this on the customer's system.

Task branch 4231b has been merged to trunk as revision 11347.

Is there any remaining work or testing needed for this task?

Greg Shah wrote:

Is there any remaining work or testing needed for this task?

I reviewed all drag and drop notes from #4170-5 to #4170-21 with the 4GL code and could not reproduce the working drag an drop 4GL program on my test VM. I used MS Tree View 6.0 control with 32bit Progress version. The hitTest doesn't return a valid node but must do according to the reviewed notes:

DEFINE VAR C-Win AS WIDGET-HANDLE NO-UNDO.

/* Definitions of handles for OCX Containers                            */
DEFINE VARIABLE CtrlFrame AS WIDGET-HANDLE NO-UNDO.
DEFINE VARIABLE chCtrlFrame AS COMPONENT-HANDLE NO-UNDO.

DEFINE VARIABLE hDragNode     AS COM-HANDLE NO-UNDO.

chCtrlFrame:TreeView:OLEDragMode = 1.

chCtrlFrame:TreeView:OLEDropMode = 1.

PROCEDURE CtrlFrame.TreeView.OLEStartDrag :
        DEFINE INPUT-OUTPUT PARAMETER Data AS COM-HANDLE. 
        DEFINE INPUT-OUTPUT PARAMETER AllowedEffects AS INTEGER.
DEF VAR xPos AS DECIMAL.
DEF VAR yPos AS DECIMAL.
DEF VAR pnt AS INTEGER EXTENT 2.
RUN getMousePoint(INPUT C-Win, OUTPUT xPos, OUTPUT yPos).

ASSIGN hDragNode = COM-SELF:HitTest(xPos AS FLOAT ,yPos AS FLOAT).

IF NOT hDragNode = 0 THEN
DO:
MESSAGE "OLEStartDrag " "Data=" Data " AllowedEffects=" AllowedEffects " xPos=" xPos " yPos=" yPos.
END.

END PROCEDURE.

PROCEDURE ScreenToClient EXTERNAL "user32":
  DEFINE INPUT PARAMETER  hwnd AS LONG.
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

PROCEDURE GetCursorPos EXTERNAL "user32":
  DEFINE INPUT PARAMETER  mpPoint AS MEMPTR.
END PROCEDURE.

PROCEDURE getMousePoint :
  DEFINE INPUT  PARAMETER pcObject AS HANDLE     NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeXpos  AS DECIMAL    NO-UNDO.
  DEFINE OUTPUT PARAMETER pdeYpos  AS DECIMAL    NO-UNDO.

  DEFINE VARIABLE mpLong  AS MEMPTR     NO-UNDO.

  SET-SIZE(mpLong) = 8.

  RUN getCursorPos(INPUT mpLong).

  RUN screenToClient(INPUT pcObject:HWND, INPUT mpLong).

  ASSIGN 
    pdeXpos = DECIMAL(GET-LONG(mpLong,1))
    pdeYpos = DECIMAL(GET-LONG(mpLong,5))
   .
END PROCEDURE.

I committed this 4GL program uast/treeview/test-treeview-sp6-3.w rev 2225 that followed the notes from #4170-5 to #4170-21. It still doesn't work with the 4GL where MS Tree View SP 6.0 32bit control is installed.

Sergey Ivanovskiy wrote:

I committed this 4GL program uast/treeview/test-treeview-sp6-3.w rev 2225 that followed the notes from #4170-5 to #4170-21. It still doesn't work with my Progress VM environment where MS Tree View SP 6.0 32bit control is installed.

I succeeded with another version of MS Tree View 32bit control, ver 5.0 SP2, although the hitTest with this version returned always the same node.

I committed rev 2226 uast/treeview/test-treeview-ver5-sp2-1.w the 4GL UI manual drag&drop test for MS Tree View 32bit control ver 5.0 SP2 from comctl32.ocx. The hitTest with this version returns always the first added tree node. It is difficult to proceed father without having a working UI test case for drag&drop.

Please look into the customer application code which must be made to work. Ask Roger (using email) where it can be found.

OK. I will search OLEStartDrag events.

Sergey Ivanovskiy wrote:

I committed rev 2226 uast/treeview/test-treeview-ver5-sp2-1.w the 4GL UI manual drag&drop test for MS Tree View 32bit control ver 5.0 SP2 from comctl32.ocx. The hitTest with this version returns always the first added tree node. It is difficult to proceed father without having a working UI test case for drag&drop.

Committed revision 2227 that fixed uast/treeview/test-treeview-ver5-sp2-1.w for MS Tree View 32bit control ver 5.0 SP2. Now this test case works properly in the 4GL. The key point is that hitTest uses different measurement units, twips (https://en.wikipedia.org/wiki/Twip) but it gets pixels on its input (see #4712-23)

Hynek, please review the committed revision 11402 (3821c) that added CHILD, NEXT, PREVIOUS, CHILDREN, FirstSibling, LastSibling node's attributes. These attributes are from INode used by MS Tree View 32bit control ver 5.0 SP2 (comctl32.ocx). Working on DropHighlight.

Please review the committed revision 11403 (3821c) that implemented drop highlight node functionality. Now working on uast/treeview/test-treeview-ver5-sp2-1.w to review and test the drag and drop functionality.

Code review 3821c revision 11402.

TreeNodeResource.getChildren, nodes must be checked for null.

TreeNodeResource.getFirstSiblingNode and getLastSiblingNode, the methods don't handle the case when there are no sibling nodes (this is the only child node).

Code review 3821c revision 11403.

TreeWidgetBase.getDropHighlight, the method uses Integer.MIN_VALUE config.dropHighlightNodeId as the default (or unknown) value, however the actual default in the config is 0.

Should we support a conversion of chCtrlFrame:TreeView:HWND? It is supposed that chCtrlFrame:TreeView is mapped to the widget handle variable hTreeView. Thus, this code can be converted into hTreeView.unwrapWidget().getHWND().

Hynek Cihlar wrote:

Code review 3821c revision 11403.

TreeWidgetBase.getDropHighlight, the method uses Integer.MIN_VALUE config.dropHighlightNodeId as the default (or unknown) value, however the actual default in the config is 0.

These found issues in the review 11402 and the review 11403 were fixed in the committed rev. 11405 (3821c) that added conversion changes in order to convert these expressions hCtrlFrame:TreeView:HWND, hNode:CHILD, hNode:PREVIOUS, hNode:NEXT to hTreeView.unwrapTreeView().getHWND(), hNode.unwrapTreeNodeFace().getChildNode(),..

With this version rev 11405 (3821c)

        /* don't drag if there are child nodes */
        IF VALID-HANDLE(hDragNode:child) THEN 
        DO:
            hDragNode = ?.
            RETURN.
        END.

            if ((new handle(hDragNode.unwrapTreeNodeFace().getChildNode()).isValid()).booleanValue())
            {
               hDragNode.setUnknown();
               returnNormal();
            }

Sorry, I missed TreeNodeResource.this.getChildNode(). Fixed it in the committed rev 11407 (3821c).

1. The drophighlight node is slightly shifted from the mouse pointer when the selected node is dragged.
2. The cursor type is incorrect. Now the dragging cursor looks like the editor's cursor.
3. It needs to review GET-MOSE-POSITION() and ThinClient.hitTest and similar methods findTreeNodeByRelativeCoordinates.
   This video test-treeview-ver5-sp2-1.mkv shows these issues.

Code review 3821c revision 11405.

getFirstSiblingNode and getLastSiblingNode still don't consider the case when this is the only node (parent.nodes.size() == 1). In this case this node is returned as the first or last sibling.

Code review 3821c revision 11407. The changes are good.

I occidentally checked that two test cases related to image-list widget aren't compiled with the current version 11447 of 3821c.
treeview/test-image-list-1.w and treeview/test-image-list-2.w:

    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/treeview/TestImageList1.java:349: error: not a statement
    [javac]          (ImageList) hTreeView.unwrapTreeView().setImageList(hImageList.unwrapImageList());
    [javac]          ^
    [javac] /home/sbi/projects/testcases/uast/src/com/goldencode/testcases/treeview/TestImageList2.java:450: error: not a statement
    [javac]          (ImageList) hTreeView.unwrapTreeView().setImageList(hImageList.unwrapImageList());
    [javac]          ^
    [javac] 2 errors

   /**
    * {@inheritDoc}
    */
   @ComMethod(name = "ADD")
   public comhandle add(ComParameter index, ComParameter key, ComParameter picture)
   {
      return add((integer) index.getValue(), (character) key.getValue(), (comhandle) picture.getValue());
   }

         chPicture.assign(com.goldencode.p2j.ui.ocx.PictureObject.loadPicture("Y:\\\\4gl\\\\testcases\\\\treeview\\\\Hand1.ico"));
         chImage.assign(hImageList.unwrapImageList().getListImages().unwrapListImages().add(new ComParameter(1), new ComParameter("image1"), new ComParameter(chPicture)));
         chExpPicture.assign(com.goldencode.p2j.ui.ocx.PictureObject.loadPicture("Y:\\\\4gl\\\\testcases\\\\treeview\\\\Hand2.ico"));
         chExpImage.assign(hImageList.unwrapImageList().getListImages().unwrapListImages().add(new ComParameter(2), new ComParameter("image2"), new ComParameter(chExpPicture)));

Can this task be put into Test mode?

Roger: I think you may have found some related behavior that seems wrong. Can you open a new task or tasks for that?

Greg Shah wrote:

Roger: I think you may have found some related behavior that seems wrong. Can you open a new task or tasks for that?

The drag and drop issues are related to the icon changing during manipulation. Would that be part of this task?

Please provide details as a separate task.

Yes, the image list tests are working in general. There is a known conversion issue that this code with a corresponding hint file

DEFINE VARIABLE hImgList  AS COM-HANDLE no-undo.

hImgList = chCtrlFrame:ImageList.

chCtrlFrame-2:TreeView:ImageList=hImgList.

chCtrlFrame-2:TreeView:ImageList=chCtrlFrame:ImageList.