FWD » Core Development » User Interface

Here is the test procedure that leads to Exception:

def button but label "button".
def frame f but.

def sub-menu sm
  menu-item mi1 label "mi1".
  menu-item mi2 label "mi2".

def menu ssm
  menu-item mm label "Elthon".
  sub-menu sm label "John".

assign but:popup-menu = menu ssm:handle.

enable all with frame f.

wait-for choose of but. 

There is just one BUTTON with popup menu assigned. When I click RMB over this BUTTON - NullPointerException is raised:

    Caused by: java.lang.NullPointerException: The drawing owner is not set. Unbalanced select/release window calls?
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.releaseWindow(AbstractGuiDriver.java:1739)
        at com.goldencode.p2j.ui.client.gui.driver.GuiOutputManager.releaseWindow(GuiOutputManager.java:294)
        at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:13879)
        at com.goldencode.p2j.ui.chui.ThinClient.applyWorker(ThinClient.java:13624)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:10947)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10455)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10409)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) 

If we add menu-item mm:accelerator = "PAGE-DOWN". statement to this procedure it will work. We can just get /menu/popup_simple.p procedure and comment out this statement there, than such procedure will reproduce described error. I've founded that if at least one accelerator is assigned to some menu item than popup menu will work.

Task branch 2966a is created yesterday (11049 trunk rev).

Current error is the following:

Caused by: java.lang.IllegalStateException: Widget sm 9 is not attached to a TopLevelWindow instance.
    at com.goldencode.p2j.ui.client.widget.AbstractWidget.topLevelWindow(AbstractWidget.java:315)
    at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:14266)
    at com.goldencode.p2j.ui.client.gui.driver.MouseHandler.handleMouseEvent(MouseHandler.java:249)
    at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.handleMouseEvent(AbstractGuiDriver.java:2559)
    at com.goldencode.p2j.ui.client.TopLevelWindow.processEvent(TopLevelWindow.java:717)

Can it be related with the latest enhancement when menu implementation was moved to separate OverlayWindow? May be we should overwrite topLevelWindow() for menu widgets if it needed?

Vadim Gindin wrote:

Can it be related with the latest enhancement when menu implementation was moved to separate OverlayWindow?

Yes, this is likely.

The primary question is why is the top level window not available in the click handler. Possibly because the overlay window with the sub-menu closes before the handler is executed (?).

Hynek Cihlar wrote:

Vadim Gindin wrote:

Can it be related with the latest enhancement when menu implementation was moved to separate OverlayWindow?

Yes, this is likely.

The primary question is why is the top level window not available in the click handler. Possibly because the overlay window with the sub-menu closes before the handler is executed (?).

Not exactly. Have a look at the SubMenuGuiImpl.mousePressed() and SubMenuGuiImpl.mouseClicked(). There are calls window() that returned not OverlayWindow but WindowGuiImpl, so I've changed them to topLevelWindow() calls. In other words for menu classes call window() will return incorrect window.. May be it will be more correctly to override window() method?

Vadim Gindin wrote:

See the change.

Yes, the change is correct.

Vadim Gindin wrote:

I've faced new bug, related to new drawing:

Sizes of popup and sub-menu are equal to Progress, but we see and white extra space by the right of menu body.
Could you advice me why it can happen..?

From the drawing hard to say, what is wrong. Does it happen in trunk or the task branch only? Please post the test case.

Hynek, the test is in the first note. The bug is also happen in trunk.

Vadim Gindin wrote:

Hynek, the test is in the first note. The bug is also happen in trunk.

This seems to be caused by your changes in trunk revision 11053.

Hynek, the test is in the first note. The bug is also happen in trunk.

This seems to be caused by your changes in trunk revision 11053.

Vadim: see if you can track the cause down and put the fix into 2966a. But it needs to be done quickly. If you can't find the cause within the next day, we may move this bug into another task.

I've fixed that. Please review the task. Current revision is 11056.

I've fixed that. Please review the task. Current revision is 11056.

When you say "that", do you mean you have fixed the drawing issue you described in note 9?

Greg Shah wrote:

I've fixed that. Please review the task. Current revision is 11056.

When you say "that", do you mean you have fixed the drawing issue you described in note 9?

Yes, I fixed the bug described in the note 9.

Code Review Task Branch 2966a Revision 11057

1. Please rebase.

2. Add history entries.

There is no revision 11059. I think you never did a bzr bind before committing your update. Please do a bzr push --overwrite ... using your current branch and then bzr bind so your changes will be automatically applied in the future.

By the way are you sure, that "blinking" bug is appeared from commit 11053?

I'm not sure what you are referencing by "blinking" bug. Are you talking about the issue in note 7?

Hynek's review of the note 7 issue found that it started in 11053 of trunk. Do you believe this is incorrect?

I'll check it.

I was checking that several times. As I can see the error appeared first time in the revision 11042..

I would suppose that the reason of the error is in SwingMouseHandler.lastHoveredWidget field. It contains some menu widget reference, for example SUB-MENU. After the menu is detached this field should be reset to null. What do you think?

Vadim Gindin wrote:

I was checking that several times. As I can see the error appeared first time in the revision 11042..

I would suppose that the reason of the error is in SwingMouseHandler.lastHoveredWidget field. It contains some menu widget reference, for example SUB-MENU. After the menu is detached this field should be reset to null. What do you think?

I though this problem was already fixed. Should I look at it?

At this moment the problem exists in trunk 11058. Hynek, have a look please.

Vadim: please take this, Hynek has other items that are higher priority.

Caused by: java.lang.RuntimeException: No renderer is registered for id = -23
    at com.goldencode.p2j.ui.client.gui.driver.GuiPrimitivesImpl.getWindowEmulator(GuiPrimitivesImpl.java:208)
    at com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver.moveToTop(SwingGuiDriver.java:503)
    at com.goldencode.p2j.ui.client.WindowManager.moveToTop(WindowManager.java:324)
    at com.goldencode.p2j.ui.client.TopLevelWindow.processEvent(TopLevelWindow.java:760)
    at com.goldencode.p2j.ui.client.gui.OverlayWindow.processEvent(OverlayWindow.java:613)
    at com.goldencode.p2j.ui.chui.ThinClient.processProgressEvent(ThinClient.java:15899)
    at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:15374)
    at com.goldencode.p2j.ui.chui.ThinClient.pop(ThinClient.java:14385)
    at com.goldencode.p2j.ui.chui.ThinClient.eventBracket(ThinClient.java:14368)
    at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:14286)
    at com.goldencode.p2j.ui.chui.ThinClient.applyWorker(ThinClient.java:14042)

1. The first one probably related with SwingMouseHandler.lastHoveredWidget. This flag should be reset each time the window is detached. Where is the better place for that? May be right in SwingGuiDriver?
2. The second one is probably caused because of the WINDOW_ACTIVATED event processing. Here is the following execution sequence:
   WINDOW_ACTIVATED event is created
   WINDOW is detached
   WINDOW_ACTIVATED is processed and exception happen.
   May be we should check window status each time WINDOW_ACTIVATED event is processed?

Another case:
Will be it correct to remove window from WindowManager.WorkArea.windows list when AbstractGuiDriver.deregisterWindow() is called?

Vadim Gindin wrote:

I need some help. There are 2 exceptions possible. The first of them is described in the note 3. The second one is the following:
[...]

1. The first one probably related with SwingMouseHandler.lastHoveredWidget. This flag should be reset each time the window is detached. Where is the better place for that? May be right in SwingGuiDriver?

lastHoveredWidget should be considered valid only when lastHoveredWidget.isDisplayed() is true, maybe it is not enforced in all cases? When isDisplayed() is used there is no need to reset the field when window destroyed, IMHO.

1. The second one is probably caused because of the WINDOW_ACTIVATED event processing. Here is the following execution sequence:
   WINDOW_ACTIVATED event is created
   WINDOW is detached
   WINDOW_ACTIVATED is processed and exception happen.
   May be we should check window status each time WINDOW_ACTIVATED event is processed?

Yes, I think we should be prepared for the possibility that the window may not be displayed at the time the dispatcher is executed and guard all the sensitive calls of the subjected window.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I need some help. There are 2 exceptions possible. The first of them is described in the note 3. The second one is the following:
[...]

1. The first one probably related with SwingMouseHandler.lastHoveredWidget. This flag should be reset each time the window is detached. Where is the better place for that? May be right in SwingGuiDriver?

lastHoveredWidget should be considered valid only when lastHoveredWidget.isDisplayed() is true, maybe it is not enforced in all cases? When isDisplayed() is used there is no need to reset the field when window destroyed, IMHO.

1. The second one is probably caused because of the WINDOW_ACTIVATED event processing. Here is the following execution sequence:
   WINDOW_ACTIVATED event is created
   WINDOW is detached
   WINDOW_ACTIVATED is processed and exception happen.
   May be we should check window status each time WINDOW_ACTIVATED event is processed?

Yes, I think we should be prepared for the possibility that the window may not be displayed at the time the dispatcher is executed and guard all the sensitive calls of the subjected window.

Or enqueue the "WINDOW is detached" on the event queue and reorder the sequence to
WINDOW_ACTIVATED event is created
enqueue WINDOW detach
WINDOW_ACTIVATED is processed
process WINDOW detach.

Third type of error:

Caused by: java.lang.IllegalStateException: Widget  19 is not attached to a TopLevelWindow instance.
    at com.goldencode.p2j.ui.client.widget.AbstractWidget.topLevelWindow(AbstractWidget.java:315)
    at com.goldencode.p2j.ui.client.TopLevelWindow.processEvent(TopLevelWindow.java:714)
    at com.goldencode.p2j.ui.client.gui.OverlayWindow.processEvent(OverlayWindow.java:613)
    at com.goldencode.p2j.ui.chui.ThinClient.processProgressEvent(ThinClient.java:15744)
    at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:15374)

Current situation is the following. I'd fixed lastHoveredWidget.isDisplayed(). Now it returns false as it must return and corresponding exception is not thrown.

When SUB-MENU body is become hidden does corresponding OverlayWindow is destroyed or just become invisible? The same for MENU?

Vadim Gindin wrote:

Current situation is the following. I'd fixed lastHoveredWidget.isDisplayed(). Now it returns false as it must return and corresponding exception is not thrown.

When SUB-MENU body is become hidden does corresponding OverlayWindow is destroyed or just become invisible? The same for MENU?

When it becomes hidden it is destroyed, see TopLevelWindow.close().

I've fixed all exceptions. The only one bug is remained. When navigating inside popup menu: sm -> mi -> (back to) sm menu is closed itself. I've also rebased the branch and removed debugging code. Please make intermediate review. Current branch revision is 11074.

I need some help.

The problem is the following. I have 2 nested opened SUB-MENUs. I'm navigating inside the body of parent sub-menu. When child SUB-MENU loses focus - it must be closed. At that moment WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning that body. But additionally new WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning parent sub-menu's body! It happen in the following code WindowManager.processWindowActivated():

      ...
      // only deacticate currently app-activated windows when activating a non-P2J window or
      // when the currently driver-active window is the owner of the new active window and
      // the new active window doesn't share activation state with its owner
      if (actDriverWindow != null &&
          (activated == null || !isSharedActivationState(actDriverWindow, activated)))
      {
         // first send deactivation events for all active windows 
         forEachActiveWindow(actDriverWindow, w -> 
         {
            if (w == activated)
            {
               // stop iteration the active windows when the currently app-active window
               // is also the new driver-active window.
               return false;
            }

            ThinClient.getInstance().postOSEvent(new WindowActivated(w, false, activated == null));

            // continue iteration
            return true;
         });
      }
      ...

#. It is not correct for menus. We should not close parent sub-menu body at all.
#. Such approach could be a reason of sub-menus bodies "blinking".

My question is what to do in this situation to not to brake some other cases? How to properly integrate menus in this mechanism?

P.S. Current testcase is simple_sm.p. I'm opening "SM2 Submenu" then move mouse pointer to it's child sub-menu "SM0 Submenu" and then move back to menu-item "Mi2 item". I expect that only "SM0 Submenu"'s body will be closed but both bodies are closed.

1. In some cases current sub-menu body must be closed only itself.
2. In some cases current sub-menu body must be closed with all opened parent's sub-menus.

At first sight I would suppose to exclude sub-menu's windows from this processing especially because of these windows are closed directly from SubMenuGuiImpl.hideBody() method. What do you think?

Vadim Gindin wrote:

I need some help.

The problem is the following. I have 2 nested opened SUB-MENUs. I'm navigating inside the body of parent sub-menu. When child SUB-MENU loses focus - it must be closed. At that moment WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning that body. But additionally new WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning parent sub-menu's body! It happen in the following code WindowManager.processWindowActivated():
[...]
By the way activated is null here.

When activated is null, it means the method handles the case when a non-P2J window is being activated, in which case all overlay windows should be closed. Check why activated is null if it is not expected to be. Also check whether close()/hide()/destroy() is called on the overlay window of the "SM2 Submenu".

It looks like this code tries to deactivate all windows for all opened sub-menus bodies and than activate the current one later in this method.

If activated is really null, then there is no window activation in the method. Only window deactivations.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I need some help.

The problem is the following. I have 2 nested opened SUB-MENUs. I'm navigating inside the body of parent sub-menu. When child SUB-MENU loses focus - it must be closed. At that moment WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning that body. But additionally new WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning parent sub-menu's body! It happen in the following code WindowManager.processWindowActivated():
[...]
By the way activated is null here.

When activated is null, it means the method handles the case when a non-P2J window is being activated, in which case all overlay windows should be closed. Check why activated is null if it is not expected to be. Also check whether close()/hide()/destroy() is called on the overlay window of the "SM2 Submenu".

What is non-P2J window? Are the menus overlay windows are non-P2J? May be there are some other non-P2J windows. If yes - what are they? The test simple_sm.p contains only main window and sub-menus bodies windows.

I don't know about activated - is the null value expected or not in my case. I'd described the case: nested sub-menu is closing. parent sub-menu should stay open. Should activated point to parent sub-menu or not? How it was designed?

It looks like this code tries to deactivate all windows for all opened sub-menus bodies and than activate the current one later in this method.

If activated is really null, then there is no window activation in the method. Only window deactivations.

Probably only deactivations. OK, but I'd expected that for sub-menus cases only nested sub-menu overlay window will be deactivated - not both (with parent). I'm just trying to understand how it was designed to work with sub-menus windows?

By the way, why there is to places for deactivation: that method and SubMenuGuiImpl.hideBody()? It looks excess.

P.S. I remember some discrussion about "mousePressed" flag. I don't know if it already known bug, but I'd found that when mouse pointer is moved from menubar sub-menu's title to it's body, than sub-menu's title loses "pressed" state, but it must be stay "pressed".

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I need some help.

The problem is the following. I have 2 nested opened SUB-MENUs. I'm navigating inside the body of parent sub-menu. When child SUB-MENU loses focus - it must be closed. At that moment WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning that body. But additionally new WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning parent sub-menu's body! It happen in the following code WindowManager.processWindowActivated():
[...]
By the way activated is null here.

When activated is null, it means the method handles the case when a non-P2J window is being activated, in which case all overlay windows should be closed. Check why activated is null if it is not expected to be. Also check whether close()/hide()/destroy() is called on the overlay window of the "SM2 Submenu".

What is non-P2J window? Are the menus overlay windows are non-P2J? May be there are some other non-P2J windows. If yes - what are they? The test simple_sm.p contains only main window and sub-menus bodies windows.

Non-P2J window is any window not belonging to the P2J client Java process.

I don't know about activated - is the null value expected or not in my case. I'd described the case: nested sub-menu is closing. parent sub-menu should stay open. Should activated point to parent sub-menu or not? How it was designed?

I would say yes, activated should point to the sub-menu not being closed. It should not be null, as this should indicate that a non-P2J window is being a activated.

The design is as follows. All menu windows are instances of OverlayWindow. The overlay windows form a window child-owner hierarchy (see TopLevelWindow.getOwner()). When a new child sub-menu is opened, it will not deactivate its owner. When owner is activated (for example through a user input), all child windows are deactivated, closed and destroyed. Similarly when close() is called, all children are deactivated, closed and destroyed. When a non-P2J window is activated, all sub-menu windows are deactivated, closed and destroyed.

It looks like this code tries to deactivate all windows for all opened sub-menus bodies and than activate the current one later in this method.

If activated is really null, then there is no window activation in the method. Only window deactivations.

Probably only deactivations. OK, but I'd expected that for sub-menus cases only nested sub-menu overlay window will be deactivated - not both (with parent). I'm just trying to understand how it was designed to work with sub-menus windows?

By the way, why there is to places for deactivation: that method and SubMenuGuiImpl.hideBody()? It looks excess.

These are not duplications. SMGI.hideBody() takes care of detaching the body widgets from the closing overlay window. The body itself does not get destroyed when the window gets closed, only the window.

P.S. I remember some discrussion about "mousePressed" flag. I don't know if it already known bug, but I'd found that when mouse pointer is moved from menubar sub-menu's title to it's body, than sub-menu's title loses "pressed" state, but it must be stay "pressed".

If mousePressed is supposed to stay true, but doesn't, then it is a bug and must be fixed.

Hynek, I stuck with that. Could you help me..

1. It seems WindowManager.processWindowActivated() identifies what to do (ACTIVE or DEACTIVATE) by condition activated != null. Right?

2. Therefore, if I'll transfer activated=ParentSM that method will not deactivate child window...

3. Parent/Child SUB-MENU's windows are always share activation state (in sense of WindowManager.isSharedActivationState() method. Is it allways true?

4. Let's consider menu key navigation in a child sub-menu.
If we press right arrow or Enter we must close both parent/child sub-menus => state (see point 3) is really shared.
If we press left arrow we must close only child sub-menu => state is not shared
Did I make correct conclusions about state sharing? If yes, we should add some state sharing information to that mechanism and set it inside key event processing. What do you think?

5. Other question. I.e. WindowManager only closes sub-menu bodies windows, but SubMenu.hide() - detaches it's children.
Does the first each time lead to the second?
Does the second each time lead to the first?

Vadim Gindin wrote:

Hynek, I stuck with that. Could you help me..

1. It seems WindowManager.processWindowActivated() identifies what to do (ACTIVE or DEACTIVATE) by condition activated != null. Right?

Yes, activated controls the method's behavior. The method process a window activation in the system (note that window deactivation is always succeeded by a window activation, whether a P2J or non-P2J window is activated). When activated is null, a non-P2J window is being activated, otherwise a P2J window is being activated. While activated controls the ACTIVE part, the DEACTIVATE part happens regardless (when the P2J client process has an active window).

2. Therefore, if I'll transfer activated=ParentSM that method will not deactivate child window...

If you activate the parent SubMenu, all children should be deactivated. processWindowActivated() checks the shared activation state and since there is no activation share between the parent and the children (the activation state is directional) the children will be deactivated. Do you see a different behavior?

3. Parent/Child SUB-MENU's windows are always share activation state (in sense of WindowManager.isSharedActivationState() method. Is it allways true?

Yes.

4. Let's consider menu key navigation in a child sub-menu.
If we press right arrow or Enter we must close both parent/child sub-menus => state (see point 3) is really shared.
If we press left arrow we must close only child sub-menu => state is not shared

Sorry, did you mean key navigation in the window menu bar or a context menu?

Did I make correct conclusions about state sharing? If yes, we should add some state sharing information to that mechanism and set it inside key event processing. What do you think?

5. Other question. I.e. WindowManager only closes sub-menu bodies windows, but SubMenu.hide() - detaches it's children.
Does the first each time lead to the second?

Yes.

Does the second each time lead to the first?

Yes.

Hynek Cihlar wrote:

...
3. Parent/Child SUB-MENU's windows are always share activation state (in sense of WindowManager.isSharedActivationState() method. Is it allways true?

Yes.

4. Let's consider menu key navigation in a child sub-menu.
If we press right arrow or Enter we must close both parent/child sub-menus => state (see point 3) is really shared.
If we press left arrow we must close only child sub-menu => state is not shared

Sorry, did you mean key navigation in the window menu bar or a context menu?

I meant window menubar. But as I remember the same behavior should be in a context menu too.

Vadim Gindin wrote:

Hynek Cihlar wrote:

...
3. Parent/Child SUB-MENU's windows are always share activation state (in sense of WindowManager.isSharedActivationState() method. Is it allways true?

Yes.

4. Let's consider menu key navigation in a child sub-menu.
If we press right arrow or Enter we must close both parent/child sub-menus => state (see point 3) is really shared.
If we press left arrow we must close only child sub-menu => state is not shared

Sorry, did you mean key navigation in the window menu bar or a context menu?

I meant window menubar. But as I remember the same behavior should be in a context menu too.

In each case you have to find the correct window that must be closed. When right key is pressed, you get the reference to the root submenu window and call close() on it. This will also close all child windows. When left key is pressed, you get the reference to the submenu window which contained the highlight before the key was processed and call close() on it. Does it make sense?

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

...
3. Parent/Child SUB-MENU's windows are always share activation state (in sense of WindowManager.isSharedActivationState() method. Is it allways true?

Yes.

4. Let's consider menu key navigation in a child sub-menu.
If we press right arrow or Enter we must close both parent/child sub-menus => state (see point 3) is really shared.
If we press left arrow we must close only child sub-menu => state is not shared

Sorry, did you mean key navigation in the window menu bar or a context menu?

I meant window menubar. But as I remember the same behavior should be in a context menu too.

In each case you have to find the correct window that must be closed. When right key is pressed, you get the reference to the root submenu window and call close() on it. This will also close all child windows. When left key is pressed, you get the reference to the submenu window which contained the highlight before the key was processed and call close() on it. Does it make sense?

OK. You mean it should work with current mechanism in WindowManager. Probably. I'll check it.

Returning windowDeactivated, as I understand when WindowManager.windowDeactivated() is called for ChildSm I'll transfer ParentSm in further to processWindowActivated() call as activated value. How to determine ParentSm there?

Vadim Gindin wrote:

How to determine ParentSm there?

To get the parent sub menu use the MenuElement.getParentSubMenu(). MenuElement is implemented by SubMenu class.

Hynek Cihlar wrote:

Vadim Gindin wrote:

How to determine ParentSm there?

To get the parent sub menu use the MenuElement.getParentSubMenu(). MenuElement is implemented by SubMenu class.

I meant WindowManager.windowDeactivated() method.. Proposed way seems not very good there.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

How to determine ParentSm there?

To get the parent sub menu use the MenuElement.getParentSubMenu(). MenuElement is implemented by SubMenu class.

I meant WindowManager.windowDeactivated() method.. Proposed way seems not very good there.

Sorry, I am not following. You want to determine a parent sub menu in WindowManager.windowDeactivated()?

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

How to determine ParentSm there?

To get the parent sub menu use the MenuElement.getParentSubMenu(). MenuElement is implemented by SubMenu class.

I meant WindowManager.windowDeactivated() method.. Proposed way seems not very good there.

Sorry, I am not following. You want to determine a parent sub menu in WindowManager.windowDeactivated()?

Yes. Parent sub-menu should be activated when child sub-menu is closed (deactivated), so I want to set activated=ParentSm and transfer it to WindowManager.processWindowActivated()

Vadim Gindin wrote:

Yes. Parent sub-menu should be activated when child sub-menu is closed (deactivated), so I want to set activated=ParentSm and transfer it to WindowManager.processWindowActivated()

The parent sub menu (or the parent window) should stay active and get focus when you call close(). There should be no need to handle the activation explicitly.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I need some help.

The problem is the following. I have 2 nested opened SUB-MENUs. I'm navigating inside the body of parent sub-menu. When child SUB-MENU loses focus - it must be closed. At that moment WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning that body. But additionally new WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning parent sub-menu's body! It happen in the following code WindowManager.processWindowActivated():
[...]
By the way activated is null here.

When activated is null, it means the method handles the case when a non-P2J window is being activated, in which case all overlay windows should be closed. Check why activated is null if it is not expected to be. Also check whether close()/hide()/destroy() is called on the overlay window of the "SM2 Submenu".

It looks like this code tries to deactivate all windows for all opened sub-menus bodies and than activate the current one later in this method.

If activated is really null, then there is no window activation in the method. Only window deactivations.

Hynek, I've checked, the methods close()/hide()/destroy() are called for OverlayWindow containing the body of SM2 Submenu.

activated is null. Why? See WindowManager.windowDeactivated(). It always transfer null for activated parameter.

You'd adviced at some moment that in specified case we should transfer ParentSM to activated parameter.. That's why I'd asked how to determine it.

P.S. Another question. Is it OK, that when SM0 Submenu is opened WindowManager.windowDeactivated() is called for window of SM2 Submenu body?

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I need some help.

The problem is the following. I have 2 nested opened SUB-MENUs. I'm navigating inside the body of parent sub-menu. When child SUB-MENU loses focus - it must be closed. At that moment WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning that body. But additionally new WINDOW_ACTIVATED(activated=false) event is sent to OverlayWindow owning parent sub-menu's body! It happen in the following code WindowManager.processWindowActivated():
[...]
By the way activated is null here.

When activated is null, it means the method handles the case when a non-P2J window is being activated, in which case all overlay windows should be closed. Check why activated is null if it is not expected to be. Also check whether close()/hide()/destroy() is called on the overlay window of the "SM2 Submenu".

It looks like this code tries to deactivate all windows for all opened sub-menus bodies and than activate the current one later in this method.

If activated is really null, then there is no window activation in the method. Only window deactivations.

Hynek, I've checked, the methods close()/hide()/destroy() are called for OverlayWindow containing the body of SM2 Submenu.

activated is null. Why? See WindowManager.windowDeactivated(). It always transfer null for activated parameter.

Do you see this on your workspace changes? If so, please commit them or send a diff and I will check it.

You'd adviced at some moment that in specified case we should transfer ParentSM to activated parameter.. That's why I'd asked how to determine it.

This should happen with the current logic.

P.S. Another question. Is it OK, that when SM0 Submenu is opened WindowManager.windowDeactivated() is called for window of SM2 Submenu body?

It is important to distinguish the two levels of window activation - the driver and the application. While at the driver, we can have only one active window at a time (it receives input focus and input from the OS), at the application level, we may have multiple active windows at a time (the overlay-window use cases).

I don't know what is the use case you refer to, but since WindowManager.windowDeactivated() implements only the driver part of window activation the execution of WindowManager.windowDeactivated() you see is most likely correct.

I've committed my changes with debugging information and rebased the branch. Now current revision is 11081. I recall, the testing scenario is the following (simple_sm.p): Open "SM2 Submenu" then move mouse pointer to it's child sub-menu "SM0 Submenu" and then move back to menu-item "Mi2 item". I expect that only "SM0 Submenu"'s body will be closed but both bodies are closed.

It looks that error happen because of SubMenuGuiImpl.hideBodies(parent(), null) call in MenuItemGuiImpl.onFocusGained(). But If I remove this call nested SM0 Submenu is not being closed when I move mouse pointer to neighbor menu-item.

It's hard to debug it because when window focus is switched to debugger - that triggers menus closing in itself.

Why all WindowActivated events have the same id=-1?

Hynek, have you try to debug?

Need advice how to debug it..

I was seing the error each time. I've applied your change but id didn't helped: I still see the error each time.

Please post your standard output (just after the problem appears and with my changes).

Vadim Gindin wrote:

Here it is.

I don't find anything wrong with this output actually. Would you be able to record a short video sequence of the error? I want to make sure we both track the same use case. Also if you could send the un-shortened standard output of that run with milliseconds time stamp in each entry, that would help, too.

Vadim Gindin wrote:

Why all WindowActivated events have the same id=-1?

This is fine, WA doesn't use the id field.

Vadim Gindin wrote:

I've made a gif and error log with time stamp added to debugging information. Have a look

Do you have by any chance yet some uncommitted changes in your working copy?

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've made a gif and error log with time stamp added to debugging information. Have a look

Do you have by any chance yet some uncommitted changes in your working copy?

Only your change and debugging code.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Here it is

Please apply the attached diff and retest.

It solves the problem for menubar. Is it temporary solution?

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

Here it is

Please apply the attached diff and retest.

It solves the problem for menubar.

So it doesn't solve it for the sub-menu issue you posted in the gif?

Is it temporary solution?

Yes, this is not the final change.

Hynek Cihlar wrote:

So it doesn't solve it for the sub-menu issue you posted in the gif?

The last change solves the problem for simple_sm.p. I've just checked it for popup_ext.p. It works too!

Vadim Gindin wrote:

Hynek Cihlar wrote:

So it doesn't solve it for the sub-menu issue you posted in the gif?

The last change solves the problem for simple_sm.p. I've just checked it for popup_ext.p. It works too!

Vadim, you can leave this issue on me. And since this is Swing-only I believe this issue doesn't have to be blocked because of this problem.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

So it doesn't solve it for the sub-menu issue you posted in the gif?

The last change solves the problem for simple_sm.p. I've just checked it for popup_ext.p. It works too!

Vadim, you can leave this issue on me.

I meant, "you can leave this problem on me".

Should I commit the your changes and debugging code?

Vadim Gindin wrote:

Should I commit the your changes and debugging code?

No, no need to. Actually you can remove all the debug changes done to track the unwanted menu hiding.

I've fixed mousePressed bug. Current revision is 11084. I'm switching to other task. Hynek, thank's for help!

Code Review Task Branch 2966a Revision 11084

The changes look good. Some minor things:

1. Please rebase.

2. Remove the import com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver; from MenuGuiImpl. MenuGuiImpl is common code and should not directly reference any of the specific drivers.

3. Remove the commented code in the SwingEmulatedWindow constructor.

4. Please add back the blank line between the imports and the class javadoc in SwingEmulatedWindow.

Done. Current branch revision is 11086 (rebased and containing code cleanup).

Code Review Task Branch 2966a Revision 11086

It looks good.

Please test it:

1. Using MAJIC runtime regression testing on devsrv01. Although most changes are GUI-only, there are a few changes that do affect common code.

2. Please execute all of the manual standalone GUI tests, for both Swing and web.

Report the test results here.

Greg Shah wrote:

Code Review Task Branch 2966a Revision 11086

It looks good.

Please test it:

1. Using MAJIC runtime regression testing on devsrv01. Although most changes are GUI-only, there are a few changes that do affect common code.

2. Please execute all of the manual standalone GUI tests, for both Swing and web.

Report the test results here.

Greg, you want me to test the change in spite of the fact that the bug "unexpected sub-menu close" is not fixed?

Vadim Gindin wrote:

Greg Shah wrote:

Code Review Task Branch 2966a Revision 11086

It looks good.

Please test it:

1. Using MAJIC runtime regression testing on devsrv01. Although most changes are GUI-only, there are a few changes that do affect common code.

2. Please execute all of the manual standalone GUI tests, for both Swing and web.

Report the test results here.

Greg, you want me to test the change in spite of the fact that the bug "unexpected sub-menu close" is not fixed?

Vadim, it is Swing-related, it shouldn't prevent you from testing Web client.

Vadim Gindin wrote:

I've faced with conversion error during conversion standalone GUI tests:

Is it known bug?

I don't see this error. Do you have any changes in proto_window_size_truncation.p? Is your testcases/uast up to date?

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've faced with conversion error during conversion standalone GUI tests:

Is it known bug?

I don't see this error. Do you have any changes in proto_window_size_truncation.p? Is your testcases/uast up to date?

I didn't change proto_window_size_truncation.p.
testcases/uast is up to date.
I'm digging.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've faced with conversion error during conversion standalone GUI tests:

Is it known bug?

I don't see this error. Do you have any changes in proto_window_size_truncation.p? Is your testcases/uast up to date?

By the way I see the error also in trunk.. You "don't see this error" - you've meant that you've ran conversion and it has finished succesfully?

Vadim Gindin wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've faced with conversion error during conversion standalone GUI tests:

Is it known bug?

I don't see this error. Do you have any changes in proto_window_size_truncation.p? Is your testcases/uast up to date?

By the way I see the error also in trunk.. You "don't see this error" - you've meant that you've ran conversion and it has finished succesfully?

Right, I did ant all on clean latest trunk and reconverted with no errors.

I got compile error when building converted code for the test browse/gui/brws-hscoll-col1.p:

    [javac] /home/vig/projects/testcases/uast/src/com/goldencode/testcases/browse/gui/BrwsHscollCol1.java:59: error: cannot find symbol
    [javac]                   tt.setF5(concat("col5 ", valueOf(i)));
    [javac]                     ^
    [javac]   symbol:   method setF5(character)
    [javac]   location: variable tt of type Buf
    [javac] /home/vig/projects/testcases/uast/src/com/goldencode/testcases/browse/gui/BrwsHscollCol1.java:60: error: cannot find symbol
    [javac]                   tt.setF6(concat("col6 ", valueOf(i)));
    [javac]                     ^
    [javac]   symbol:   method setF6(character)
    [javac]   location: variable tt of type Buf

Delete all your previously converted code. Then convert all testcases that need to be converted, in one batch. You can see problems like this if you only convert 1-2 files at a time and there are previous programs with converted temp-tables that match names.

Greg Shah wrote:

Delete all your previously converted code. Then convert all testcases that need to be converted, in one batch. You can see problems like this if you only convert 1-2 files at a time and there are previous programs with converted temp-tables that match names.

Thanks. Now it is converted successfully.

Regression testing is iddle. I'd started it yesterday morning. It should be already finished.. Logs do not contain exceptions.

runtime-regression:
     [exec] ** Clearing vig folder...
     [exec] rm: cannot remove ‘/home/vig/web*.dat’: No such file or directory
     [exec] ** Running CTRL-C part...
     [exec]

Check the client logs; I get this:

ARNING: {Reader} failure in reading loop
java.io.InvalidClassException: com.goldencode.p2j.util.BaseDataType; local class incompatible: stream classdesc serialVersionUID = -7309201695662737443, local class serialVersionUID = 1236072489269656981
        at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:621)
        at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1623)
        at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1518)
        at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1623)
        at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1518)
        at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1623)
        at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1518)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1774)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1351)
        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:371)
        at com.goldencode.p2j.ui.client.WidgetEntry.readExternal(WidgetEntry.java:116)
        at java.io.ObjectInputStream.readExternalData(ObjectInputStream.java:1840)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1799)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1351)
        at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:371)
        at com.goldencode.p2j.ui.ScreenBuffer.readExternal(ScreenBuffer.java:1038)
        at java.io.ObjectInputStream.readExternalData(ObjectInputStream.java:1840)

Did you build P2J manually? This shouldn't happen, except in cases when p2j jars are different.

Constantin Asofiei wrote:

Check the client logs; I get this:
[...]

Did you build P2J manually? This shouldn't happen, except in cases when p2j jars are different.

No didn't build P2J manually. Do you advice to do it manually?

Vadim Gindin wrote:

Constantin Asofiei wrote:

Check the client logs; I get this:
[...]

Did you build P2J manually? This shouldn't happen, except in cases when p2j jars are different.

No didn't build P2J manually. Do you advice to do it manually?

The question is removed.

I've ran standalone GUI tests. All works except the last one: browse.gui.BrowseGuiStat1. There is infinite loop:

07/28/2016 19:21:01 YEKT] (StandardServer.invoke:SEVERE) {00000043:000000CE:bogus} Abnormal end!
java.lang.IllegalArgumentException: Unknown schema:  _temp
        at com.goldencode.p2j.persist.DMOIndex.getMetadata(DMOIndex.java:741)
        at com.goldencode.p2j.persist.DMOIndex.getImplementingClass(DMOIndex.java:411)
        at com.goldencode.p2j.persist.RecordBuffer.<init>(RecordBuffer.java:1439)
        at com.goldencode.p2j.persist.TemporaryBuffer.<init>(TemporaryBuffer.java:561)

1. ACCELERATOR is not working. I faced that in some task..
2. I've faced with NullPointerException at some moment but I couldn't reproduce it again. The bug is the following. MenuItemGuiImp.requestFocus() calls super.requestFocus() and the parent call contains the following command Widget<O> currentFocus = ancestor().currentFocus();. So, ansector() returns null because it tries to find the parent of class TitledWindow and body's OverlayWindow is not descendant of that class. That should also be fixed.

Should I fix these bugs? Here or in other task?

Probably some misconfiguration.

Yes. Your testcases directory.xml probably doesn't have the _temp database activated.

Should I fix these bugs? Here or in other task?

Yes, here.

Greg Shah wrote:

Probably some misconfiguration.

Yes. Your testcases directory.xml probably doesn't have the _temp database activated.

it is activated..

I don't understand... I'm testing simple_sm.p on windev01 and faced that for menubar sub-menus "SM2 Submenu" and "Smooth operator" accelerators work differently: for the first one don't work, and for the second one work as usual..

I couldn't reproduce the NullPointerException.

I've faced with NullPointerException at some moment but I couldn't reproduce it again. The bug is the following. MenuItemGuiImp.requestFocus() calls super.requestFocus() and the parent call contains the following command Widget<O> currentFocus = ancestor().currentFocus();. So, ansector() returns null because it tries to find the parent of class TitledWindow and body's OverlayWindow is not descendant of that class. That should also be fixed.

I was wrong. OverlayWindow is a descendant of TitledWindow, therefore menuItem.ansector() must allways return not null value because it is getting through the parents: SubMenuBody->OverlayWindow (containing the body). Hynek am I right?

Vadim Gindin wrote:

I couldn't reproduce the NullPointerException.

I've faced with NullPointerException at some moment but I couldn't reproduce it again. The bug is the following. MenuItemGuiImp.requestFocus() calls super.requestFocus() and the parent call contains the following command Widget<O> currentFocus = ancestor().currentFocus();. So, ansector() returns null because it tries to find the parent of class TitledWindow and body's OverlayWindow is not descendant of that class. That should also be fixed.

I was wrong. OverlayWindow is a descendant of TitledWindow, therefore menuItem.ansector() must allways return not null value because it is getting through the parents: SubMenuBody->OverlayWindow (containing the body). Hynek am I right?

ancestor() for sub menu body may return null in case the body is not attached to a window. The result should be checked for null to prevent NPEs.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I couldn't reproduce the NullPointerException.

I've faced with NullPointerException at some moment but I couldn't reproduce it again. The bug is the following. MenuItemGuiImp.requestFocus() calls super.requestFocus() and the parent call contains the following command Widget<O> currentFocus = ancestor().currentFocus();. So, ansector() returns null because it tries to find the parent of class TitledWindow and body's OverlayWindow is not descendant of that class. That should also be fixed.

I was wrong. OverlayWindow is a descendant of TitledWindow, therefore menuItem.ansector() must allways return not null value because it is getting through the parents: SubMenuBody->OverlayWindow (containing the body). Hynek am I right?

ancestor() for sub menu body may return null in case the body is not attached to a window. The result should be checked for null to prevent NPEs.

I've committed the fix.

Vadim Gindin wrote:

Constantin Asofiei wrote:

Check the client logs; I get this:
[...]

Did you build P2J manually? This shouldn't happen, except in cases when p2j jars are different.

No didn't build P2J manually. Do you advice to do it manually?

I'd built it manually second time but it didn't helped. Second run also became idle..

Your environment seems to be broken. I don't think it is worth trying to diagnose it.

Just remove everything inside your ~/testing/ directory from devsrv01, and then run the run_regression.sh configure target to recreate it completely. You will also have to do a new build there to convert MAJIC for the first time.

Eric: Can you please review the issue in notes 90 through 92 and tell us if you have any ideas?

I don't understand... I'm testing simple_sm.p on windev01 and faced that for menubar sub-menus "SM2 Submenu" and "Smooth operator" accelerators work differently: for the first one don't work, and for the second one work as usual..

If you are looking for help here, you will need t post some details about the scenario. Consider that when testing on windev01, your keystrokes are being captured by a local app and the accelerator support in that app may be intercepting the keys before they are passed to the windows os and thus they don't get forwarded to the 4GL code.

Greg Shah wrote:

Eric: Can you please review the issue in notes 90 through 92 and tell us if you have any ideas?

This error occurs when the server tries to load the temp-table schema from the dmo-index.xml file which is generated by conversion, but the directory has been configured to not load the _temp schema. This is the long version of what you already noted.

Vadim, please post your directory.xml and the dmo-index.xml from your application jar file.

Eric Faulhaber wrote:

Greg Shah wrote:

Eric: Can you please review the issue in notes 90 through 92 and tell us if you have any ideas?

This error occurs when the server tries to load the temp-table schema from the dmo-index.xml file which is generated by conversion, but the directory has been configured to not load the _temp schema. This is the long version of what you already noted.

Vadim, please post your directory.xml and the dmo-index.xml from your application jar file.

Greg, Eric that error has gone at some moment. Sorry to trouble.

Greg Shah wrote:

I don't understand... I'm testing simple_sm.p on windev01 and faced that for menubar sub-menus "SM2 Submenu" and "Smooth operator" accelerators work differently: for the first one don't work, and for the second one work as usual..

If you are looking for help here, you will need t post some details about the scenario. Consider that when testing on windev01, your keystrokes are being captured by a local app and the accelerator support in that app may be intercepting the keys before they are passed to the windows os and thus they don't get forwarded to the 4GL code.

I have to neighbour sub-menus:

define sub-menu sm11
   menu-item mi00 label "QQQQ" accelerator "U".

define sub-menu sm2
   menu-item mii1 label "UUUUU" accelerator "U".

Vadim Gindin wrote:

Eric Faulhaber wrote:

Greg Shah wrote:

Eric: Can you please review the issue in notes 90 through 92 and tell us if you have any ideas?

This error occurs when the server tries to load the temp-table schema from the dmo-index.xml file which is generated by conversion, but the directory has been configured to not load the _temp schema. This is the long version of what you already noted.

Vadim, please post your directory.xml and the dmo-index.xml from your application jar file.

Greg, Eric that error has gone at some moment. Sorry to trouble.

I've faced it again. But you were right. There was a conflict: dmo_index.xml from p2j.jar didn't contain _temp schema description, but actual file dmo_index.xml located in /testcases/uast/src/... was correct. Now I've ran tested browse test and tested it. It also works.

Vadim Gindin wrote:

There was a conflict: dmo_index.xml from p2j.jar didn't contain _temp schema description, but actual file dmo_index.xml located in /testcases/uast/src/... was correct.

I'm not sure what you mean. There shouldn't be any dmo_index.xml file in p2j.jar at all. This file should only exist as a resource in the application jar file.

Eric Faulhaber wrote:

Vadim Gindin wrote:

There was a conflict: dmo_index.xml from p2j.jar didn't contain _temp schema description, but actual file dmo_index.xml located in /testcases/uast/src/... was correct.

I'm not sure what you mean. There shouldn't be any dmo_index.xml file in p2j.jar at all. This file should only exist as a resource in the application jar file.

In my case that file got into p2j.jar erroneously (in testcases package) and this caused an error I'd faced with.

I have to neighbour sub-menus:
[...]
Now I've found that 'U' works for menu-item "UUUUU" and don't work for "QQQQ". It probably works as default mnemonic but not as accelerator..

I can imagine multiple possibilities:

1. You cannot have multiple accelerators that use the same key. If this is the case, perhaps the last one specified, is the one that is active.

2. If an accelerator key is not present in the label of the menu, then it is invalid since there is no way to give the user feedback about what the accelerator is.

Please write testcases to check these two theories.

Greg Shah wrote:

I have to neighbour sub-menus:
[...]
Now I've found that 'U' works for menu-item "UUUUU" and don't work for "QQQQ". It probably works as default mnemonic but not as accelerator..

I can imagine multiple possibilities:

1. You cannot have multiple accelerators that use the same key. If this is the case, perhaps the last one specified, is the one that is active.

Don't work

2. If an accelerator key is not present in the label of the menu, then it is invalid since there is no way to give the user feedback about what the accelerator is.

It is correct, but what to test here? I'd tried to assign some accelerator to each menu-item in sub-menu. Anyway the didn't work.

As I can see, accelerators stopped working at all.. Did the customer update OpenEdge last time?

As I can see, accelerators stopped working at all.. Did the customer update OpenEdge last time?

No, I don't think they changed anything.

Perhaps your VNC client is broken?

Greg Shah wrote:

As I can see, accelerators stopped working at all.. Did the customer update OpenEdge last time?

No, I don't think they changed anything.

Perhaps your VNC client is broken?

Probably. I use Remmina. There is a button "Grab all keyboard events (CTRL+R)". I've tried both cases: pressed and unpressed but unsuccessfully.

Could you try this testcase:

define sub-menu sm11
   menu-item mi00 label "QQQQ" accelerator "PAGE-UP".

define sub-menu sm2
   menu-item mii1 label "UUUUU" accelerator "UP".
   menu-item mii2 label "UUUUU" accelerator "CTRL-E".
   menu-item mii3 label "UUUUU" accelerator "E".
   menu-item mii4 label "UUUUU" accelerator keylabel(keycode("del")).
   menu-item mii5 label "UUUUU" accelerator "CTRL-W".
   menu-item mii6 label "UUUUU" accelerator keylabel(keycode("CTRL-B")).

define menu m1 menubar
  sub-menu sm2 label "SM2 Su&bme&nu".
  sub-menu sm11 label "Smooth operator".
  menu-item exit label "E&x&it".

assign current-window:menubar = menu m1:handle.

on choose of menu-item mi00
   message "mi00".

on choose of menu-item mii1
   message "mii1".

on choose of menu-item mii2
   message "mii2".

on choose of menu-item mii3
   message "mii3".

on choose of menu-item mii4
   message "mii4".

on choose of menu-item mii5
   message "mii5".

on choose of menu-item mii6
   message "mii6".

def frame f1.
enable all with frame f1.

wait-for choose of menu-item exit.

I've probably found a reason: accelerators work only if menus are closed. Strange..

I've probably found a reason: accelerators work only if menus are closed. Strange..

Interesting.

OK, please investigate the different scenarios noted above. Then fix our deviations, including the disabling of accelerators when the menus are open.

Please document your findings and the plan to resolve the issues. This needs to be closed soon.

The only one bug is remained: transferring "pressed" state to neighbour in the menubar.

Vadim Gindin wrote:

The only one bug is remained: transferring "pressed" state to neighbour in the menubar.

Can you finish this today?

Greg Shah wrote:

Vadim Gindin wrote:

The only one bug is remained: transferring "pressed" state to neighbour in the menubar.

Can you finish this today?

That's not very simple thing, but, yes, I think so.

Vadim Gindin wrote:

Vadim Gindin wrote:

Greg Shah wrote:

Vadim Gindin wrote:

The only one bug is remained: transferring "pressed" state to neighbour in the menubar.

Can you finish this today?

That's not very simple thing, but, yes, I think so.

Unfortunately. I'm stuck trying to solve "pressing state transferring" problem. It is the following. When user presses some menubar item (sub-menu) and move mouse pointer to its neighbour (also menubar item) then we must display new item as pressed. Have a look at the following image:

If if trajectory of mouse pointer is as shown in this image we must transfer "pressed" state to sub-menu "Smooth operator". I have a problem with this scenario trying to define target widget. "pressed" state transferring is happen in mouseExited methods of MenuItemGuiImpl and SubMenuGuiImpl. There is a call in MenuItemGuiImpl (when mouse is moved from menu-item "mii1" to sub-menu "Smooth Operator"):
[...]
It should be a target sub-menu "Smooth operator", but it become SubMenuBody of the first sub-menu "SM2 Submenu".

Yes, this makes sense since the menu item the mouse pointer us just leaving is attached to a top-level window which is not related to "Smooth Operator". I am thinking whether this logic should be performed in mouseEntered().

#In mouseEntered we don't know the widget we came from. It is also cannot be solved using focusing, because not all widgets are focusable..

#I've also guessed that mouseExited is not called because of moving out from the body window (at least related to that). Is the previous window saved somewhere? Probably no, but it seems it should do that and use from mouseExited somehow. What do you think?

Vadim Gindin wrote:

#In mouseEntered we don't know the widget we came from. It is also cannot be solved using focusing, because not all widgets are focusable..

Do we need to know the widget we came from?

#I've also guessed that mouseExited is not called because of moving out from the body window

I believe mouseExited is supposed to be called when leaving top-level window, if not there is a problem somewhere.

Hynek Cihlar wrote:

Vadim Gindin wrote:

#In mouseEntered we don't know the widget we came from. It is also cannot be solved using focusing, because not all widgets are focusable..

Do we need to know the widget we came from?

Yes, we need to get "pressed" flag from it and set it for new widget.

#I've also guessed that mouseExited is not called because of moving out from the body window

I believe mouseExited is supposed to be called when leaving top-level window, if not there is a problem somewhere.

1. Source and target widgets are from different windows. I'm searching target widget calling findMouseSource() from some window object. I don't know for sure at that moment should I search target widget in the same window or in some other. If in other how to find out - concrete window object. May be SwingMouseHandler.findMouseSource(e) is more appropriate?
2. Ok. I'm trying to find out why it isn't called. What about coordinates from MouseEvent. I've specified concrete example earlier. I suspect that these coordinates are relative to active window (body window). If yes, I'll not be able to determine target widget in other window using these coordinates..

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

#In mouseEntered we don't know the widget we came from. It is also cannot be solved using focusing, because not all widgets are focusable..

Do we need to know the widget we came from?

Yes, we need to get "pressed" flag from it and set it for new widget.

Can we track the pressed state for the whole menu? Then the exited widget won't be needed.

#I've also guessed that mouseExited is not called because of moving out from the body window

I believe mouseExited is supposed to be called when leaving top-level window, if not there is a problem somewhere.

1. Source and target widgets are from different windows. I'm searching target widget calling findMouseSource() from some window object. I don't know for sure at that moment should I search target widget in the same window or in some other. If in other how to find out - concrete window object. May be SwingMouseHandler.findMouseSource(e) is more appropriate?

It would be better not to rely on findMouseSource(e), you would have to figure out the correct window and this could be tricky.

I've specified concrete example earlier. I suspect that these coordinates are relative to active window (body window). If yes, I'll not be able to determine target widget in other window using these coordinates..

findMouseSource() takes coordinates relative to the container physical location, so screen location must be converted before they can be passed in.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

#In mouseEntered we don't know the widget we came from. It is also cannot be solved using focusing, because not all widgets are focusable..

Do we need to know the widget we came from?

Yes, we need to get "pressed" flag from it and set it for new widget.

Can we track the pressed state for the whole menu? Then the exited widget won't be needed.

#I've also guessed that mouseExited is not called because of moving out from the body window

I believe mouseExited is supposed to be called when leaving top-level window, if not there is a problem somewhere.

1. Source and target widgets are from different windows. I'm searching target widget calling findMouseSource() from some window object. I don't know for sure at that moment should I search target widget in the same window or in some other. If in other how to find out - concrete window object. May be SwingMouseHandler.findMouseSource(e) is more appropriate?

It would be better not to rely on findMouseSource(e), you would have to figure out the correct window and this could be tricky.

I've specified concrete example earlier. I suspect that these coordinates are relative to active window (body window). If yes, I'll not be able to determine target widget in other window using these coordinates..

findMouseSource() takes coordinates relative to the container physical location, so screen location must be converted before they can be passed in.

It's a good idea to store "pressed" state of an active menu item. I'm trying.

P.S.
There are also posting FOCUS_GAINED and events in mouseExited methods, so I can't fully get rid of findMouseSource() there at least at this moment.

Vadim Gindin wrote:

P.S.
There are also posting FOCUS_GAINED and events in mouseExited methods, so I can't fully get rid of findMouseSource() there at least at this moment.

Focus can be managed in the mouseEntered() of the entering widget, instead of focusing the entering widget from the exiting one, what do you think?

Hynek Cihlar wrote:

Vadim Gindin wrote:

P.S.
There are also posting FOCUS_GAINED and events in mouseExited methods, so I can't fully get rid of findMouseSource() there at least at this moment.

Focus can be managed in the mouseEntered() of the entering widget, instead of focusing the entering widget from the exiting one, what do you think?

1. The first one is the scenario, described in the note 117. The mouse pointer is moved from body window to main window to a "Smooth operator" sub-menu. Here we must not to reset mbarPressedState flag in MenuGuiImpl, because we'll use it to initialize "Smooth operator" pressed state during this move.
2. The second scenario is simpler. After application is started we opening "Smooth operator" sub-menu and moving mouse pointer down through the opened body little lower than the body and click in the free space. Body is closed and we must reset mbarPressedState flag.

In the former scenario we must not to reset the flag, but in the latter we must reset it. In both we move mouse pointer from a body window to a main window. I don't understand how to distinguish these scenarios. Have a look please.

Vadim Gindin wrote:

1. The first one is the scenario, described in the note 117. The mouse pointer is moved from body window to main window to a "Smooth operator" sub-menu. Here we must not to reset mbarPressedState flag in MenuGuiImpl, because we'll use it to initialize "Smooth operator" pressed state during this move.
2. The second scenario is simpler. After application is started we opening "Smooth operator" sub-menu and moving mouse pointer down through the opened body little lower than the body and click in the free space. Body is closed and we must reset mbarPressedState flag.

The pressed state should be cleared on two events, when the main window (which owns the menu) is deactivated or when focus is moved to a widget not belonging to the menu.

The window deactivation is important in cases focus is moved out of P2J client to another process in the system or when another top-level P2J window is activated.

I believe this should resolve both the cases you mentioned.

Hynek Cihlar wrote:

or when focus is moved to a widget not belonging to the menu.

This is a bit tricky. I think the most reliable way to figure this one out is to monitor mouse clicks for the window owning the menu and to consider any click outside of the menu as focus moved out of the menu.

Hynek Cihlar wrote:

Hynek Cihlar wrote:

or when focus is moved to a widget not belonging to the menu.

This is a bit tricky. I think the most reliable way to figure this one out is to monitor mouse clicks for the window owning the menu and to consider any click outside of the menu as focus moved out of the menu.

But we must also track clicks outside of the P2J window, that also must lead to flag reset.

Vadim Gindin wrote:

But we must also track clicks outside of the P2J window, that also must lead to flag reset.

That's what the window deactivation handling is for. When you click outside, the window gets deactivated (and the menu pressed state should be reset). Note that the window can be deactivated also by other means than mouse clicks, that's why we should not track mouse events but really the window de-activation events.

Hynek Cihlar wrote:

Vadim Gindin wrote:

But we must also track clicks outside of the P2J window, that also must lead to flag reset.

That's what the window deactivation handling is for. When you click outside, the window gets deactivated (and the menu pressed state should be reset). Note that the window can be deactivated also by other means than mouse clicks, that's why we should not track mouse events but really the window de-activation events.

I've committed changes (WindowGuiImpl, SwingMouseHandler). Please review. It works, I'm not sure if SMH is a good place.

Vadim Gindin wrote:

I've committed changes (WindowGuiImpl, SwingMouseHandler). Please review. It works, I'm not sure if SMH is a good place.

Code review 2966a revisions 11096..11098.

Unless you see any good reasons not to do this, please save the reference of the pressed menu item (not just the boolean flag) in Menu/MenuGuiImpl or in root SubMenu/SubMenuGuiImpl for context menus. Create static methods in Menu/MenuGuiImpl getPressed() and setPressed() that will return the pressed MenuElement from Menu or root SubMenu for a menu bar or context menu respectively. This change will remove the need for storing the actual state on multiple places.

Please remove MenuElementGuiImpl, since pressed state can be managed by Menu/MenuGuiImpl, the methods setPressed() and isPressed() are not needed.

SubMenuBody should not implement MenuElementGuiImpl. MenuElement and SubMenuBody don't share their concerns and so should not be mixed. While MenuElement interface solves the disconnected physical structure of the menu items, SubMenuBody implements the widget container for the menu widgets.

The reset of pressed state on mouse click should not be done in SwingMouseHandler as this behavior is too specific for a concrete widget. If possible, please move the logic to WindowGuiImpl.processEvent() or maybe use the mechanism of MouseWidgetAction. Beware that MouseWidgetAction would require some tweaking.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've committed changes (WindowGuiImpl, SwingMouseHandler). Please review. It works, I'm not sure if SMH is a good place.

Code review 2966a revisions 11096..11098.

Unless you see any good reasons not to do this, please save the reference of the pressed menu item (not just the boolean flag) in Menu/MenuGuiImpl or in root SubMenu/SubMenuGuiImpl for context menus. Create static methods in Menu/MenuGuiImpl getPressed() and setPressed() that will return the pressed MenuElement from Menu or root SubMenu for a menu bar or context menu respectively. This change will remove the need for storing the actual state on multiple places.

Please remove MenuElementGuiImpl, since pressed state can be managed by Menu/MenuGuiImpl, the methods setPressed() and isPressed() are not needed.

SubMenuBody should not implement MenuElementGuiImpl. MenuElement and SubMenuBody don't share their concerns and so should not be mixed. While MenuElement interface solves the disconnected physical structure of the menu items, SubMenuBody implements the widget container for the menu widgets.

The reset of pressed state on mouse click should not be done in SwingMouseHandler as this behavior is too specific for a concrete widget. If possible, please move the logic to WindowGuiImpl.processEvent() or maybe use the mechanism of MouseWidgetAction. Beware that MouseWidgetAction would require some tweaking.

I don't understand. "pressed" state transferring makes sense only for MENUBAR..

Vadim Gindin wrote:

I don't understand. "pressed" state transferring makes sense only for MENUBAR..

To make sure, we are on the same ground. How the "pressed" state translates to the UI behavior in MENUBAR?

Hynek Cihlar wrote:

Vadim Gindin wrote:

I don't understand. "pressed" state transferring makes sense only for MENUBAR..

To make sure, we are on the same ground. How the "pressed" state translates to the UI behavior in MENUBAR?

pressed flag relates only to MENUBAR item (MENU-ITEM or SUB-MENU) drawing when it is focused (mouse entered or focused by key navigation). If pressed is true, then item title is drawn concave. If pressed is false then title is drawn convex. Body show for sub-menu works through mouseClicked event.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I don't understand. "pressed" state transferring makes sense only for MENUBAR..

To make sure, we are on the same ground. How the "pressed" state translates to the UI behavior in MENUBAR?

pressed flag relates only to MENUBAR item (MENU-ITEM or SUB-MENU) drawing when it is focused (mouse entered or focused by key navigation). If pressed is true, then item title is drawn concave. If pressed is false then title is drawn convex. Body show for sub-menu works through mouseClicked event.

I've haste with that answer a little. There is some confusion with that flag.

Vadim Gindin wrote:

I've haste with that answer a little. There is some confusion with that flag.

Yes, there seem to be some duplicated and dead code in MenuItemGuiImpl. Assuming MenuItemGuiImpl is never a menu bar element, the pressed state and the related logic in MenuItemGuiImpl can be safely removed as it is only used in the context of menubar drawing. This will simplify things a bit.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've haste with that answer a little. There is some confusion with that flag.

Yes, there seem to be some duplicated and dead code in MenuItemGuiImpl. Assuming MenuItemGuiImpl is never a menu bar element, the pressed state and the related logic in MenuItemGuiImpl can be safely removed as it is only used in the context of menubar drawing. This will simplify things a bit.

Not exactly. MenuItemGuiImpl can be menubar item. Strange logic I mentioned yesterday was about recursive sub-menu pressed setting logic (setSubmenuPressed) and so on. I'm debugging.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I've haste with that answer a little. There is some confusion with that flag.

Yes, there seem to be some duplicated and dead code in MenuItemGuiImpl. Assuming MenuItemGuiImpl is never a menu bar element, the pressed state and the related logic in MenuItemGuiImpl can be safely removed as it is only used in the context of menubar drawing. This will simplify things a bit.

Not exactly. MenuItemGuiImpl can be menubar item. Strange logic I mentioned yesterday was about recursive sub-menu pressed setting logic (setSubmenuPressed) and so on. I'm debugging.

I've committed my changes. I'm debugging the following problem. When I open the first sub-menu "SM2 Submenu" and open the first "SM0 Submenu", but move mouse pointer from the title "SM0 Submenu" down to menu-item "Mi2 i&tem" and all bodies being closed but only "SM0 Submenu" body must being closed.

I've found that WINDOW_DEACTIVATED is generated for "SM2 Submenu" body window (but it must-not be generated) from WindowManager.processWindowActivated(). It happen because of main window being activated. I don't know why. Here is the stack trace, printed by LogHelper from WindowActivated constructor for the main window.

com.goldencode.p2j.ui.client.event.WindowActivated.<init>(WindowActivated.java:59)
com.goldencode.p2j.ui.client.WindowManager.processWindowActivated(WindowManager.java:1543)
com.goldencode.p2j.ui.client.WindowManager.windowActivated(WindowManager.java:1360)
com.goldencode.p2j.ui.client.gui.driver.swing.SwingEmulatedWindow$2.windowActivated(SwingEmulatedWindow.java:314)
java.awt.AWTEventMulticaster.windowActivated(AWTEventMulticaster.java:389)
java.awt.AWTEventMulticaster.windowActivated(AWTEventMulticaster.java:389)
java.awt.Window.processWindowEvent(Window.java:2070)
javax.swing.JFrame.processWindowEvent(JFrame.java:305)
java.awt.Window.processEvent(Window.java:2017)
java.awt.Component.dispatchEventImpl(Component.java:4881)
java.awt.Container.dispatchEventImpl(Container.java:2292)
java.awt.Window.dispatchEventImpl(Window.java:2750)
java.awt.Component.dispatchEvent(Component.java:4703)
java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1954)
java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:995)
java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:488)
java.awt.Component.dispatchEventImpl(Component.java:4752)
java.awt.Container.dispatchEventImpl(Container.java:2292)
java.awt.Window.dispatchEventImpl(Window.java:2750)
java.awt.Component.dispatchEvent(Component.java:4703)
java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
java.awt.EventQueue.access$500(EventQueue.java:97)
java.awt.EventQueue$3.run(EventQueue.java:709)
java.awt.EventQueue$3.run(EventQueue.java:703)

Vadim Gindin wrote:

I don't know how to debug further. Could you help me.

This sounds like the problem we've had already. Please try to apply the diff in note 66 and check whether it will go away. Also please test web client.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I don't know how to debug further. Could you help me.

This sounds like the problem we've had already. Please try to apply the diff in note 66 and check whether it will go away. Also please test web client.

You were right. I'd moved forward a little and almost fixed key navigation after "press" flag refactoring. Now I've faced with the following bug. I press ALT, than arrow-down - the first menu become opened and the first item from it is shown selected. I press arrow-rigth and expect that the "Smooth operator" will be opened. It happen, but it is closed right after it was shown. WINDOW_DEACTIVATED is thrown there. Could you have a look why.

P.S. I'd temporarily committed your changes from the note 66. Current revision (rebased and contained the last key nav changes is 11105).

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I don't know how to debug further. Could you help me.

This sounds like the problem we've had already. Please try to apply the diff in note 66 and check whether it will go away. Also please test web client.

You were right. I'd moved forward a little and almost fixed key navigation after "press" flag refactoring. Now I've faced with the following bug. I press ALT, than arrow-down - the first menu become opened and the first item from it is shown selected. I press arrow-rigth and expect that the "Smooth operator" will be opened. It happen, but it is closed right after it was shown. WINDOW_DEACTIVATED is thrown there. Could you have a look why.

I will look at this.

P.S. I'd temporarily committed your changes from the note 66. Current revision (rebased and contained the last key nav changes is 11105).

I've been looking for a final solution of this issue, too.

Hynek Cihlar wrote:

P.S. I'd temporarily committed your changes from the note 66. Current revision (rebased and contained the last key nav changes is 11105).

I've been looking for a final solution of this issue, too.

I've committed a fix for this into 2966a (rev 11106). I have tested the fix with the uast GUI tests (including the menu cases) as well as the customer's GUI application. So far I am positive this solution could work OK. Vadim, please update and test the menu cases.

Hynek, I've checked the last test with key navigation. The second menu body is still closing right after opening..

Vadim Gindin wrote:

The second menu body is still closing right after opening..

I don't see this. Do you have maybe any uncommitted changes in your working copy? Can you provide me with the P2J client output?

Hynek Cihlar wrote:

Vadim Gindin wrote:

The second menu body is still closing right after opening..

I don't see this. Do you have maybe any uncommitted changes in your working copy? Can you provide me with the P2J client output?

Yes, sorry. For key navigation there is 2 separate cases: "pressed" state (means only pressed title) and "body displayed" state. I've added new corresponding static flag denoting the fact of body displaying. Now after I press right-arrow the second sub-menu body is shown and closed right away. Please have a look.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

The second menu body is still closing right after opening..

I don't see this. Do you have maybe any uncommitted changes in your working copy? Can you provide me with the P2J client output?

Yes, sorry. For key navigation there is 2 separate cases: "pressed" state (means only pressed title) and "body displayed" state. I've added new corresponding static flag denoting the fact of body displaying. Now after I press right-arrow the second sub-menu body is shown and closed right away. Please have a look.

Looks good. Please clean up the changes and let's do a review if there are no other issues left.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

The second menu body is still closing right after opening..

I don't see this. Do you have maybe any uncommitted changes in your working copy? Can you provide me with the P2J client output?

Yes, sorry. For key navigation there is 2 separate cases: "pressed" state (means only pressed title) and "body displayed" state. I've added new corresponding static flag denoting the fact of body displaying. Now after I press right-arrow the second sub-menu body is shown and closed right away. Please have a look.

Looks good. Please clean up the changes and let's do a review if there are no other issues left.

1. I'd asked about a previous bug noted in 140. The body of sub-menu "Smooth operator" must be opened after I press right-arrow on the first item in the first sub-menu "SM2 Submenu". But it is opened and closed right away instead... It is still actual and can be reproduced with my latest changes 11107. So please look there.
2. When body is displayed through key navigation the first item is not always drawn as selected. I'm working on it.

Vadim Gindin wrote:

1. I'd asked about a previous bug noted in 140. The body of sub-menu "Smooth operator" must be opened after I press right-arrow on the first item in the first sub-menu "SM2 Submenu". But it is opened and closed right away instead... It is still actual and can be reproduced with my latest changes 11107. So please look there.

Ok, I see. I'm on it.

Hynek Cihlar wrote:

Vadim Gindin wrote:

1. I'd asked about a previous bug noted in 140. The body of sub-menu "Smooth operator" must be opened after I press right-arrow on the first item in the first sub-menu "SM2 Submenu". But it is opened and closed right away instead... It is still actual and can be reproduced with my latest changes 11107. So please look there.

Ok, I see. I'm on it.

Vadim, I have committed a fix in revision 11108. The unexpected submenu hiding is resolved, however this uncovers some unrelated focusing issues.

Hynek Cihlar wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

1. I'd asked about a previous bug noted in 140. The body of sub-menu "Smooth operator" must be opened after I press right-arrow on the first item in the first sub-menu "SM2 Submenu". But it is opened and closed right away instead... It is still actual and can be reproduced with my latest changes 11107. So please look there.

Ok, I see. I'm on it.

Vadim, I have committed a fix in revision 11108. The unexpected submenu hiding is resolved, however this uncovers some unrelated focusing issues.

Hynek, "Smooth operator" sub-menu is now shown, but WINDOW_DEACTIVATED for this sub-menu is still thrown. It must not be thrown in our case and it lead to erroneous consequences.. (SubMenuGuiImpl.hideBody() call, destroying listener call, wrong flags setting in MenuGuiImpl.

Vadim Gindin wrote:

Hynek, "Smooth operator" sub-menu is now shown, but WINDOW_DEACTIVATED for this sub-menu is still thrown. It must not be thrown in our case and it lead to erroneous consequences.. (SubMenuGuiImpl.hideBody() call, destroying listener call, wrong flags setting in MenuGuiImpl.

I'll look at it. If you have any uncommitted changes, please commit them.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek, "Smooth operator" sub-menu is now shown, but WINDOW_DEACTIVATED for this sub-menu is still thrown. It must not be thrown in our case and it lead to erroneous consequences.. (SubMenuGuiImpl.hideBody() call, destroying listener call, wrong flags setting in MenuGuiImpl.

I'll look at it. If you have any uncommitted changes, please commit them.

I've already committed some additional debugging info. Current revision is 11109.

Vadim Gindin wrote:

I'll look at it. If you have any uncommitted changes, please commit them.

I've already committed some additional debugging info. Current revision is 11109.

See a fix in 11110.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I'll look at it. If you have any uncommitted changes, please commit them.

I've already committed some additional debugging info. Current revision is 11109.

See a fix in 11110.

The branch is rebased. Current revision is 11113.

Is there anything left to work in this task?

Yes. Key navigation is not fully fixed.

How quickly can this be done?

Greg Shah wrote:

How quickly can this be done?

I hope I'll finish it tomorrow

How quickly can this be done?

I hope I'll finish it tomorrow

OK.

Hynek, I have a problem here about focusing as you noted earlier (note 149). What happen. After first ran I press ALT and the first menubar item (Sub-menu "SM2 Submenu") become pressed, but body is closed. I press down-arrow and the body is shown. Here is the code of the down-arrow event processing in SubMenu.processKeyEvent():

      if (key == Key.VK_ENTER                                                    ||
          key == Keyboard.SE_MENU_DROP                                           ||
          (Menu.isMenubarElement(this) && actionCode == Keyboard.KA_CURSOR_DOWN) ||
          (!Menu.isMenubarElement(this) && actionCode == Keyboard.KA_CURSOR_RIGHT))
      {
         // What if body is already displayed? In this case there will be focused item in 
         // the body, that will process this event.

         showBody();
         if (key != Keyboard.SE_MENU_DROP)
         {
            menuDropEvents();

            // we are via keys, select initial focus
            setInitialFocus();
         }

         parent().repaint();
         event.consume();
         return;
      }

submenu_body.focus = menu_item["Mi&2 item"].
bordered_panel.focus = submenu_body.
overlay_window.focus = bordered_panel.

Now have a look at AbstractWidget.hasFocus(). There is also recursive parent.hasFocus() call where parent is also defined using parent() method. Here is the call stack:

MenuItemGuiImpl.drawTitleRectangle()
  MenuItemGuiImpl.hasFocus()
     SubMenuBody.hasFocus()
        BorderedPanelGuiImpl.hasFocus()
           OverlayWindow.hasFocus
              WindowManager.isWindowActive(topLevelWindow())        returns @false@

We probably should activate body window before body drawing.

Another way is to change parent() calls to calls MenuElement.getParent() in all focusing methods for menu widgets. That will allow us to change parent hierarchy from:
MenuItem -> SubMenuBody -> BorderedPanel -> OverlayWindow
to
MenuItem -> SubMenu -> Menubar -> WindowGuiImpl
But I suppose it's harder.

What do you think?

Vadim Gindin wrote:

Hynek, I have a problem here about focusing as you noted earlier (note 149). What happen. After first ran I press ALT and the first menubar item (Sub-menu "SM2 Submenu") become pressed, but body is closed. I press down-arrow and the body is shown. Here is the code of the down-arrow event processing in SubMenu.processKeyEvent():
[...]
Here setInitialFocus() call set the first item of sub-menu as focused in the following order using parent() to get the parent widget:
[...]
Later a body drawing happen and respectively menu_item["Mi&2 item"] drawing happen too. When menu-item is drawn hasFocus() is called (see MenuItemGuiImpl.drawTitleRectangle()) and it returned false, but it must return true.

Now have a look at AbstractWidget.hasFocus(). There is also recursive parent.hasFocus() call where parent is also defined using parent() method. Here is the call stack:
[...]
In other words OverlayWindow is still not activated at the time of body item drawing...
I've checked that and found that WindowManager.windowActivated() is really called after menu-item drawing.

We probably should activate body window before body drawing.

The problem was with the window activation events not serialized through the event queue, and so the event ordering was not right. A fix in 2966a revision 11114.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek, I have a problem here about focusing as you noted earlier (note 149). What happen. After first ran I press ALT and the first menubar item (Sub-menu "SM2 Submenu") become pressed, but body is closed. I press down-arrow and the body is shown. Here is the code of the down-arrow event processing in SubMenu.processKeyEvent():
[...]
Here setInitialFocus() call set the first item of sub-menu as focused in the following order using parent() to get the parent widget:
[...]
Later a body drawing happen and respectively menu_item["Mi&2 item"] drawing happen too. When menu-item is drawn hasFocus() is called (see MenuItemGuiImpl.drawTitleRectangle()) and it returned false, but it must return true.

Now have a look at AbstractWidget.hasFocus(). There is also recursive parent.hasFocus() call where parent is also defined using parent() method. Here is the call stack:
[...]
In other words OverlayWindow is still not activated at the time of body item drawing...
I've checked that and found that WindowManager.windowActivated() is really called after menu-item drawing.

We probably should activate body window before body drawing.

The problem was with the window activation events not serialized through the event queue, and so the event ordering was not right. A fix in 2966a revision 11114.

Hynek, I still see the same. WINDOW_ACTIVATED is thrown after menu-item drawn.

Vadim Gindin wrote:

Hynek, I still see the same. WINDOW_ACTIVATED is thrown after menu-item drawn.

And do you see the same behavior?

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek, I still see the same. WINDOW_ACTIVATED is thrown after menu-item drawn.

And do you see the same behavior?

Yes, when I open sub-menu the first item is not drawn as focused, because of item.hasFocus() at the time of drawing returns false (MenuItemGuiImpl.drawTitleRectangle()). The same behavior.

SubMenuGuiImpl.showBody() is called during down-arrow event processing and it creates OverlayWindow wnd and calls wnd.show(), that respectively calls WindowManager.activateWindow();. But why in this chain the call WindowManager.windowActivated(); is absent?

Vadim Gindin wrote:

SubMenuGuiImpl.showBody() is called during down-arrow event processing and it creates OverlayWindow wnd and calls wnd.show(), that respectively calls WindowManager.activateWindow();. But why in this chain the call WindowManager.windowActivated(); is absent?

I trying to debug. I would suppose that the last change in WindowManager.windowActivated() was wrong, when ThinClient.getInstance().invokeLater() was added. It set aside WINDOW_ACTIVATED event. But we need to throw it right now.

It is complex bug.
I've removed invokeLater() back and debugged using integrated debugger in Eclipse. It worked good (WINDOW_ACTIVATED before drawing), but when I just run the test without breakpoints WINDOW_ACTIVATED is thrown after drawing.

It's just for watching.. What do you think?

Vadim Gindin wrote:

Vadim Gindin wrote:

SubMenuGuiImpl.showBody() is called during down-arrow event processing and it creates OverlayWindow wnd and calls wnd.show(), that respectively calls WindowManager.activateWindow();. But why in this chain the call WindowManager.windowActivated(); is absent?

windowActivated() is called alright. At least I can see it in my trace output.

I trying to debug. I would suppose that the last change in WindowManager.windowActivated() was wrong, when ThinClient.getInstance().invokeLater() was added. It set aside WINDOW_ACTIVATED event. But we need to throw it right now.

No, the change in windowAcivated() is actually needed. All UI events should be dispatched on the main thread, otherwise unexpected error may arise from incorrect event ordering.

It is complex bug.

Yes, indeed. Focus management in the GUI client has issues.

I've removed invokeLater() back and debugged using integrated debugger in Eclipse. It worked good (WINDOW_ACTIVATED before drawing), but when I just run the test without breakpoints WINDOW_ACTIVATED is thrown after drawing.

invokeLater() is needed, without it you will run into thread racing issues, which may work depending how your debugger affects threads' run states.

I have identified some problems which affect the menu use case, details in my next message.

Hynek Cihlar wrote:

I have identified some problems which affect the menu use case, details in my next message.

One cause is that SubMenuGuiImpl.showBody() is called twice and two overlay windows are created (and one of them is closed thereafter). However I am not sure yet, this is the main cause of the broken focus. Please investigate why the method is called twice, see the call stack below.

com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver.moveToTop(SwingGuiDriver.java:507)
com.goldencode.p2j.ui.client.WindowManager.forceActivateWindow(WindowManager.java:386)
com.goldencode.p2j.ui.client.WindowManager.activateWindow(WindowManager.java:425)
com.goldencode.p2j.ui.client.gui.OverlayWindow.show(OverlayWindow.java:246)
com.goldencode.p2j.ui.client.gui.SubMenuGuiImpl.showBody(SubMenuGuiImpl.java:1263)
com.goldencode.p2j.ui.client.gui.SubMenuGuiImpl.lambda$onFocusGained$1(SubMenuGuiImpl.java:998)
com.goldencode.p2j.ui.chui.ThinClient.waitForEvent(ThinClient.java:13475)
com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:11329)
com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:10908)
com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10856)
com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10810)
sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

Hynek Cihlar wrote:

Hynek Cihlar wrote:

I have identified some problems which affect the menu use case, details in my next message.

One cause is that SubMenuGuiImpl.showBody() is called twice and two overlay windows are created (and one of them is closed thereafter). However I am not sure yet, this is the main cause of the broken focus. Please investigate why the method is called twice, see the call stack below.

[...]

I've set breakpoint on the row wnd.show() in showBody(). I've pressed down-arrow. The run is stopped on that breakpoint. I pressed F8 and caught the following exception:

Caused by: java.lang.RuntimeException: GUI driver sent deactivation event for window not previously activated!
    at com.goldencode.p2j.ui.client.WindowManager.lambda$16(WindowManager.java:1403)
    at com.goldencode.p2j.ui.client.WindowManager$$Lambda$51/381002414.run(Unknown Source)
    at com.goldencode.p2j.ui.chui.ThinClient.waitForEvent(ThinClient.java:13465)
    at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:11327)
    at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:10906)
    at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10854)
    at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10808)

Vadim Gindin wrote:

Hynek Cihlar wrote:

Hynek Cihlar wrote:

I have identified some problems which affect the menu use case, details in my next message.

One cause is that SubMenuGuiImpl.showBody() is called twice and two overlay windows are created (and one of them is closed thereafter). However I am not sure yet, this is the main cause of the broken focus. Please investigate why the method is called twice, see the call stack below.

[...]

I've set breakpoint on the row wnd.show() in showBody(). I've pressed down-arrow. The run is stopped on that breakpoint. I pressed F8 and caught the following exception:
[...]

I've also added System.out.println() to showBody() and the only one row is in the result output..

Vadim Gindin wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

Hynek Cihlar wrote:

I have identified some problems which affect the menu use case, details in my next message.

One cause is that SubMenuGuiImpl.showBody() is called twice and two overlay windows are created (and one of them is closed thereafter). However I am not sure yet, this is the main cause of the broken focus. Please investigate why the method is called twice, see the call stack below.

[...]

I've set breakpoint on the row wnd.show() in showBody(). I've pressed down-arrow. The run is stopped on that breakpoint. I pressed F8 and caught the following exception:
[...]

I've also added System.out.println() to showBody() and the only one row is in the result output..

Try the right arrow key while a menu item highlighted to close the current sub menu and open the next sub menu on the right.

When key navigation results in new sub menu body being opened, a focus must be explicitly requested for the opened sub menu body. The focus processing before the body window is opened is only relevant for the old body window and/or the main window. See 2966a revision 11116. There are still some repaint problems and focus doesn't transfer correctly to/out from a MenuItem in a menu bar with key navigation.

Hynek Cihlar wrote:

When key navigation results in new sub menu body being opened, a focus must be explicitly requested for the opened sub menu body. The focus processing before the body window is opened is only relevant for the old body window and/or the main window. See 2966a revision 11116. There are still some repaint problems and focus doesn't transfer correctly to/out from a MenuItem in a menu bar with key navigation.

Hynek, I don't understand. What your change is fixing? When you open sub-menu body the first is the item shown selected?

Vadim Gindin wrote:

Hynek Cihlar wrote:

When key navigation results in new sub menu body being opened, a focus must be explicitly requested for the opened sub menu body. The focus processing before the body window is opened is only relevant for the old body window and/or the main window. See 2966a revision 11116. There are still some repaint problems and focus doesn't transfer correctly to/out from a MenuItem in a menu bar with key navigation.

Hynek, I don't understand. What your change is fixing? When you open sub-menu body the first is the item shown selected?

Do you also see an exception described in the note 169.

Vadim Gindin wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

When key navigation results in new sub menu body being opened, a focus must be explicitly requested for the opened sub menu body. The focus processing before the body window is opened is only relevant for the old body window and/or the main window. See 2966a revision 11116. There are still some repaint problems and focus doesn't transfer correctly to/out from a MenuItem in a menu bar with key navigation.

Hynek, I don't understand. What your change is fixing? When you open sub-menu body the first is the item shown selected?

One of the test cases is as follows. Alt is pressed, "SM2 Submenu" gets pressed, arrow down pressed, the submenu body is displayed with the focus on the first menu element ("M2 item"), arrow right pressed, "SM2 Submenu" body is closed "Smooth operator" submenu is opened with the focus on the first menu element ("MMM").

Previously when key navigation led to opening a sub menu body, focus was not properly set and so key navigation was no longer possible.

Do you also see an exception described in the note 169.

No I don't see it.

Strange.. I still see an exception and the first item in sub-menu body is not get focused when body is opened using down-arrow.

Vadim Gindin wrote:

Strange.. I still see an exception and the first item in sub-menu body is not get focused when body is opened using down-arrow.

Is the item really not focused, i.e. it doesn't react on key input, or is it only not drawn as highlighted? Do you have any uncommitted changes?

Hynek Cihlar wrote:

Vadim Gindin wrote:

Strange.. I still see an exception and the first item in sub-menu body is not get focused when body is opened using down-arrow.

Is the item really not focused, i.e. it doesn't react on key input, or is it only not drawn as highlighted? Do you have any uncommitted changes?

I had 1 change: commented out invokeLater() in WindowManager.windowActivated(). When I'd reverted it an exception is gone, but highlighting is not repaired.

Item is focused (onFocusGained method is called). It is only drawn not as highlighted.

Vadim Gindin wrote:

I had 1 change: commented out invokeLater() in WindowManager.windowActivated(). When I'd reverted it an exception is gone, but highlighting is not repaired.

Item is focused (onFocusGained method is called). It is only drawn not as highlighted.

Good we seem to be on the same ground. I didn't address the highlighting problem.

Hynek Cihlar wrote:

Vadim Gindin wrote:

I had 1 change: commented out invokeLater() in WindowManager.windowActivated(). When I'd reverted it an exception is gone, but highlighting is not repaired.

Item is focused (onFocusGained method is called). It is only drawn not as highlighted.

Good we seem to be on the same ground. I didn't address the highlighting problem.

OK ) I just recall, as I can understand, the reason is in the following. The item is not drawn as highlighted because hasFocus() returns false in spite of it is really focused. Now look at AbstractWidget.hasFocus(). It returns the result of a call WindowManager.isWindowActive(topLevelWindow()) wich is false for body OverlayWindow. When I've added debugging output to WindowManager.windowActivated() inside invokeLater() I've seen that it is called after the body with its item is drawn. That's why hasFocus returns false and it is not drawn as highlighted.

Vadim Gindin wrote:

It is 2 separate threads:
[...]
and
[...]
Full stacks are in attached files.

Yes, that is why the WM.windowActivated() logic is being enqueued on the main thread.

1. WindowManager.activatedWindow() is already called.
2. WindowManager.windowActivated() is not called yet.
3. AbstractWidget.hasFocus() calls WindowManager.isWindowActivated() wich checks activeDriverWindow.get()
4. WindowManager.activatedWindow() just moves the window to the end of window list.

1. Change criteria of window activation in AbstractWidget.hasFocus()
2. Try to refactor the code to make body drawing run after WindowManager.activatedWindow() is called.

What do you think about the first solution? May be we should check whether the body window is in the end of the list as it is happen in WindowManager.activatedWindow()?

Vadim Gindin wrote:

Once again. We have the following at the moment of menu item drawing:

1. WindowManager.activatedWindow() is already called.
2. WindowManager.windowActivated() is not called yet.
3. AbstractWidget.hasFocus() calls WindowManager.isWindowActivated() wich checks activeDriverWindow.get()
4. WindowManager.activatedWindow() just moves the window to the end of window list.

We have potential solutions:

1. Change criteria of window activation in AbstractWidget.hasFocus()
2. Try to refactor the code to make body drawing run after WindowManager.activatedWindow() is called.

What do you think about the first solution? May be we should check whether the body window is in the end of the list as it is happen in WindowManager.activatedWindow()?

The real problem is the focus processing that happens yet before the sub menu body is displayed. ThinClient.nextFocus() is used for focusing the next menu element. This leads to FocusManager.focusChange() which does some heavy focus processing which includes switching focus back and forth between the widget (menu element) with original focus to/from the target focus widget (menu element). This processing generates several intermittent FocusEvent instances leading for example to SubmenuGuiImpl.showBody() being invoked multiple times. This focus processing is needed for some widgets to allow to revoke the ENTRY event, which I think is not needed for menu elements (there is no way for a menu element to revoke an ENTRY event AFAIK).

We should cleanup this mess by replacing ThinClient.nextFocus() in MenuItem.processKeyEvent() with FocusManager.setFocusOn(). This will prevent SubMenuGuiImpl.showBody() being called multiple times showing and closing new overlay windows several times. This will also simplify the solution for the highlighting issue, which is most likely repaint not called at the right time.

This focus processing is needed for some widgets to allow to revoke the ENTRY event,

It is needed for the NO-APPLY. If I recall properly, it is also needed because when ENTRY is sent the previous widget that had the focus must still be the focused widget. For example, I think the FOCUS system handle will point to that widget.

which I think is not needed for menu elements (there is no way for a menu element to revoke an ENTRY event AFAIK).

Can menus receive ENTRY events at all?

Greg Shah wrote:

Can menus receive ENTRY events at all?

According to 4GL doc and my tests menu elements do not receive ENTRY events, that's why I think the extra focus processing is not needed for menu elements.

Hynek Cihlar wrote:

Greg Shah wrote:

Can menus receive ENTRY events at all?

According to 4GL doc and my tests menu elements do not receive ENTRY events, that's why I think the extra focus processing is not needed for menu elements.

Great! Go ahead and bypass the normal focus processing logic, which should significantly simplify things.

As I can see too, ENTRY/LEAVE events are not supported by menu elements. May be parent.nextFocus()/parent.prevFocus() instead of setFocusOn() are more appropriate.

Vadim Gindin wrote:

As I can see too, ENTRY/LEAVE events are not supported by menu elements. May be parent.nextFocus()/parent.prevFocus() instead of setFocusOn() are more appropriate.

Yes. I am about to commit the focus as well as the repaint issues.

Hynek Cihlar wrote:

Vadim Gindin wrote:

As I can see too, ENTRY/LEAVE events are not supported by menu elements. May be parent.nextFocus()/parent.prevFocus() instead of setFocusOn() are more appropriate.

Yes. I am about to commit the focus as well as the repaint issues.

Please see 2966a revision 11117. It solves the focus and repaint issues in GUI as well as in ChUI.

Note there are some remaining issues, (1) Exit not drawing its pressed state and (2) mouse entering a submenu doesn't open its body. (1) already exists in trunk, (2) is a regression of the other changes in 2966a.

Hynek Cihlar wrote:

Note there are some remaining issues, (1) Exit not drawing its pressed state and (2) mouse entering a submenu doesn't open its body. (1) already exists in trunk, (2) is a regression of the other changes in 2966a.

Also please note I kept some debugging trace statements, just in case and to be resolved before merging to trunk.

I've fixed simple_sm.p test, rebased the branch, removed debugging code and missed headers and javadoc. Current revision is 11125. Please review. Key navigation is not working for popup_ext. I'll try to fix it today. If I'll not fix it, I'll switch to #3105.

Vadim Gindin wrote:

I've fixed simple_sm.p test, rebased the branch, removed debugging code and missed headers and javadoc. Current revision is 11125. Please review. Key navigation is not working for popup_ext. I'll try to fix it today. If I'll not fix it, I'll switch to #3105.

I've fixed key navigation, but there is another bug: the first item is drawn as selected right after menu is opened. mouseEntered() is called for this item..

but there is another bug: the first item is drawn as selected right after menu is opened. mouseEntered() is called for this item..

Does this happen in trunk or is it a regression in 2966a?

Greg Shah wrote:

but there is another bug: the first item is drawn as selected right after menu is opened. mouseEntered() is called for this item..

Does this happen in trunk or is it a regression in 2966a?

It also happen in trunk. The reason is in the following. When RMB click is processed, then mouse source is calculated as the first item of menu (using call window.findMouseSource(event) in ThinClient). i.e. by click point coordinates.

Code review 2966a.

The changes look good, just some minor things.

ThinClient.waitForEvent()
Call wnd.getMenubar() only once and cache the received object.
The new Runnable() is better to be replaced with a lambda: () -> {}.

There is some commented code in SwingMouseHandler.

MenuGuiImpl
Some lines are longer than 98 chars.

MenuItemGuiImpl
drawTitleRectangle() contains commented out code.

SubMenuGuiImpl
An extra line added to history entry 013.

SubMenuGuiImpl
If I interpret the code correctly, mouseEntered() is never called with e null, but the
method javadoc and implementation suggests it could. Probably some orphaned logic?

WindowGuiImpl.setMessageText()
getGuiMessageArea() called twice, the value should be cached in a local var.

WindowGuiImpl.processEvent()
The return statement should be in brackets.

if (((KeyInput) event).isConsumed())
   return;

AbstractWidget
ansector -> ancestor
hasFocus() should probably also handle the case when this is detached from its parent.

SubMenu.hideBody()
Result of getParent() should be cached.

TopLevelWindow.processEvent()
Result of WindowManager.findWindow(this.getId().asInt()) should be cached.

Window.isMessageTextNull()
Shouldn't the method return true if messageArea null?

Also, please note that I have committed a couple of cosmetic changes in 2966a.

but there is another bug: the first item is drawn as selected right after menu is opened. mouseEntered() is called for this item..

Does this happen in trunk or is it a regression in 2966a?

It also happen in trunk. The reason is in the following. When RMB click is processed, then mouse source is calculated as the first item of menu (using call window.findMouseSource(event) in ThinClient). i.e. by click point coordinates.

Please create a new sub-task of #2677 for this bug. Since it is already in the trunk, we don't have to solve it here.

At this point, please resolve the code review feedback from Hynek and we will get this branch into testing.

Hynek Cihlar wrote:

Code review 2966a.
..
SubMenuGuiImpl
If I interpret the code correctly, mouseEntered() is never called with e null, but the
method javadoc and implementation suggests it could. Probably some orphaned logic?

Yes, there was the call mouseEntered(null) earlier.

AbstractWidget
ansector -> ancestor
hasFocus() should probably also handle the case when this is detached from its parent.

I didn't change that method at all and I didn't faced with related errors.. Do you insist on it?

TopLevelWindow.processEvent()
Result of WindowManager.findWindow(this.getId().asInt()) should be cached.

There are different parameters in these calls.

Window.isMessageTextNull()
Shouldn't the method return true if messageArea null?

yes, you're right.

I've fixed the code corresponding to latest remarks and rebased the branch. Current revision is 11131. Please review again.

I've created the separate task #3176 for the bug, described in the notes 193, 195.

Vadim Gindin wrote:

Hynek Cihlar wrote:

hasFocus() should probably also handle the case when this is detached from its parent.

I didn't change that method at all and I didn't faced with related errors.. Do you insist on it?

If you didn't touch it, just leave it as is.

TopLevelWindow.processEvent()
Result of WindowManager.findWindow(this.getId().asInt()) should be cached.

There are different parameters in these calls.

You are right. But I just realized there is another problem with the condition. If owner != null while no longer registered and this still registered then this window will be considered a main top-level window (the branch else if (WindowManager.findWindow(this.getId().asInt()) != null) will be executed) which is not correct. I am not sure whether such a case is really valid (owner not registered while this window registered), but let's be safe and make it more explicit.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

hasFocus() should probably also handle the case when this is detached from its parent.

I didn't change that method at all and I didn't faced with related errors.. Do you insist on it?

If you didn't touch it, just leave it as is.

TopLevelWindow.processEvent()
Result of WindowManager.findWindow(this.getId().asInt()) should be cached.

There are different parameters in these calls.

You are right. But I just realized there is another problem with the condition. If owner != null while no longer registered and this still registered then this window will be considered a main top-level window (the branch else if (WindowManager.findWindow(this.getId().asInt()) != null) will be executed) which is not correct. I am not sure whether such a case is really valid (owner not registered while this window registered), but let's be safe and make it more explicit.

How to process this case? Should I add owner == null && condition to the branch else if (WindowManager.findWindow(this.getId().asInt()) != null) or somehow else?

Vadim Gindin wrote:

How to process this case? Should I add owner == null && condition to the branch else if (WindowManager.findWindow(this.getId().asInt()) != null) or somehow else?

Yes, that would do it.

Hynek Cihlar wrote:

Vadim Gindin wrote:

How to process this case? Should I add owner == null && condition to the branch else if (WindowManager.findWindow(this.getId().asInt()) != null) or somehow else?

Yes, that would do it.

Done.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

How to process this case? Should I add owner == null && condition to the branch else if (WindowManager.findWindow(this.getId().asInt()) != null) or somehow else?

Yes, that would do it.

Done.

The changes look good, just please remove "null means turning on key mode." from the javadoc of SubMenuGuiImpl.mouseEntered() as it is no longer needed.

The first run has finished. The most of tests are failed and the most of failed tests (all I've seen) are failed at the stage of authorizing.. I've started the next run.

Vadim Gindin wrote:

The first run has finished. The most of tests are failed and the most of failed tests (all I've seen) are failed at the stage of authorizing.. I've started the next run.

Strange. The same result. The most of tests are failed at authorization stage. Different values are used for username. Here is an example:

wait = true; millis = ʼ180000ʼ; failing screen =                                               
      MMMMMM           MMMMMM         ##############                                           
      MMMMMMM         MMMMMMM             ###    ## ###    ### ####   ####                     
confidential info redacted
      MMMMMM           MMMMMM AAAAA   AAAAA  JJJJJJJ    IIIIIIII   CCCCCCC                     
      Copyright ................                                   
                     User Name: d                                                              
                      Password:                                                                
                User NOT Authorized to enter the MAJIC System!                                 
      Report any problems to the GSO IT Help Desk at extension: 3062.                          
Please Login to the MAJIC System.                                                              

   FAILED 180.055                                                                              
timeout before the specific screen buffer became available (Mismatched data at line 0, column 1
. Expected 'M' (0x004D at relative Y 0, relative X 1) and found ' ' (0x0000 at relative Y 0, re
lative X 1), template: screens/navigation/data_collection.txt.)    

Vadim, this looks like a side-effect from a failed scenario before this one. Please check the failed scenarios by hand and see which one fails.

Thank's Constantin. At this moment I've found gso_233 failed test. There must be:

10/06/2009                  PROJECT OPERATIONS                    21:07:35
confidential info redacted
│   Status: R (Released) Oasis Hold: N  Facility:           Mech Signoff: Y    │
confidential info redacted
│ How Mal Code: 000                                           RCA Status: N    │
└──────────────────────────────────────────────────────────────────────────────┘

(F)ind (N)ext (P)rev (A)dd (U)pdate (D)elete (O)utput (2)Skills (R)eturn
Enter choice or <SPACE> for list of additional options: F
Enter data or press F4 to end.

wait = true; millis = ʼ180000ʼ; failing screen = 
09/04/2016                  PROJECT OPERATIONS                    09:42:29
confidential info redacted
│   Status: F (Firm)     Oasis Hold: N  Facility:           Mech Signoff: Y    │
confidential info redacted
│ How Mal Code: 000                                           RCA Status: N    │
└──────────────────────────────────────────────────────────────────────────────┘
(F)ind (N)ext (P)rev (A)dd (U)pdate (D)elete (O)utput (2)Skills (R)eturn 
Enter choice or <SPACE> for list of additional options: F
Enter data or press F4 to end.

The field Status must be R (Released), but really F (Firm)

Vadim, from the 20160902_075739 result set you have 4 driver threads failed (at the GSO set) at the login_to_main_menu step - if this fails, then all tests run on that driver thread will fail, as this is a prerequisite. This is either caused by devsrv01 load or by some event processing in your changes... although your 2966a changes have only menu or GUI related changes...

From the failed login screens, looks like one or more keys are not intercepted by the SSH session. Please rebase and run only the main part again.

Constantin Asofiei wrote:

Vadim, from the 20160902_075739 result set you have 4 driver threads failed (at the GSO set) at the login_to_main_menu step - if this fails, then all tests run on that driver thread will fail, as this is a prerequisite. This is either caused by devsrv01 load or by some event processing in your changes... although your 2966a changes have only menu or GUI related changes...

From the failed login screens, looks like one or more keys are not intercepted by the SSH session. Please rebase and run only the main part again.

Also I would check for any suspicious exceptions in the client and server logs.

I'd rebased the branch yesterday and tried to run main part separately several times and all regression testing again. I got the similar error (correct syman/test123 values was passed):

wait = true; millis = ʼ180000ʼ; failing screen =                                                           
      MMMMMM           MMMMMM         ##############                                                       
      MMMMMMM         MMMMMMM             ###    ## ###    ### ####   ####                                 
confidential info redacted
      MMMMMM           MMMMMM AAAAA   AAAAA  JJJJJJJ    IIIIIIII   CCCCCCC                                 
      Copyright ................
                     User Name: man                                                                        
                      Password:                                                                            
                User NOT Authorized to enter the MAJIC System!                                             
      Report any problems to the GSO IT Help Desk at extension: 3062.                                      
Please Login to the MAJIC System.                                                                          

   FAILED 180.063                                                                                          
timeout before the specific screen buffer became available (Mismatched data at line 0, column 1. Expected '

Caused by: com.goldencode.p2j.net.SilentUnwindException: Connection ended abnormally
        at com.goldencode.p2j.util.TransactionManager$WorkArea.honorStopCondition(TransactionManager.java:5770)
        at com.goldencode.p2j.util.TransactionManager$WorkArea.access$1800(TransactionManager.java:5529)
        at com.goldencode.p2j.util.TransactionManager.popScope(TransactionManager.java:2114)
        at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:1435)
        at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:451)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:76)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:709)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:364)
        at com.goldencode.p2j.net.Conversation.run(Conversation.java:184)
        at java.lang.Thread.run(Thread.java:745)
[09/05/2016 14:43:20 EDT] (Dispatcher.processInbound():SEVERE) {0000000F:00000055:syman} Unexpected throwable.
java.lang.reflect.InvocationTargetException
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:76)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:709)

[09/05/2016 14:38:59 EDT] (Dispatcher.processInbound():SEVERE) {00000006:00000030:syman} Unexpected throwable.
java.lang.reflect.InvocationTargetException
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:76)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:709)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:364)
        at com.goldencode.p2j.net.Conversation.run(Conversation.java:184)
        at java.lang.Thread.run(Thread.java:745)
Caused by: com.goldencode.p2j.net.SilentUnwindException: Connection ended abnormally
        at com.goldencode.p2j.util.TransactionManager$WorkArea.honorStopCondition(TransactionManager.java:5770)
        at com.goldencode.p2j.util.TransactionManager$WorkArea.access$1800(TransactionManager.java:5529)
        at com.goldencode.p2j.util.TransactionManager.popScope(TransactionManager.java:2114)
        at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:1435)
        at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:451)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:76)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:709)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:364)
        at com.goldencode.p2j.net.Conversation.run(Conversation.java:184)
        at java.lang.Thread.run(Thread.java:745)
[09/05/2016 14:40:38 EDT] (Dispatcher.processInbound():SEVERE) {00000008:00000039:syman} Unexpected throwable.
java.lang.reflect.InvocationTargetException
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:76)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:709)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:364)
        at com.goldencode.p2j.net.Conversation.run(Conversation.java:184)

Vadim Gindin wrote:

I'd rebased the branch yesterday and tried to run main part separately several times and all regression testing again. I got the similar error (correct syman/test123 values was passed):
[...]
Logs contain a lot of following errors (server_gso_0.log):
[...]
or (server.log)
[...]

If you haven't, check the client log, too.

Client logs do not contain exceptions

Vadim: please remove your testing environment and recreate it. Then try running main regression again.

Ok, by the way, I've noted that I don't have permissions for ~/repo and ~/shared:

vig@devsrv01:~$ install -d ~/repo && sshfs filesrv01:/opt/code/ ~/repo
install: cannot change permissions of ‘/home/vig/repo’: Permission denied
vig@devsrv01:~$ cd ~/repo/
vig@devsrv01:~/repo$ ls -la
ls: reading directory .: Permission denied
total 0

Vadim Gindin wrote:

Ok, by the way, I've noted that I don't have permissions for ~/repo and ~/shared:
[...]
Should I run chmod for these folders? Are there some changes in environment setting process?

What are you trying to do? The repo is no longer on filesrv01, you don't need to do this.

What Greg meant was to execute run_regression.sh configure so that everything is checked out again... Also, before doing this, you can remove everything from your ~/testing folder on devsrv01 (don't forget to save results or anything else you might need).

Constantin Asofiei wrote:

Vadim Gindin wrote:

Ok, by the way, I've noted that I don't have permissions for ~/repo and ~/shared:
[...]
Should I run chmod for these folders? Are there some changes in environment setting process?

What are you trying to do? The repo is no longer on filesrv01, you don't need to do this.

Oh, certainly. I forgot about that transmission to devsrv01. Thanks.

What Greg meant was to execute run_regression.sh configure so that everything is checked out again... Also, before doing this, you can remove everything from your ~/testing folder on devsrv01 (don't forget to save results or anything else you might need).

I understand. By the way, how to correctly run main part separately?

./run_regession.sh server-startup
./run_main.sh

Vadim Gindin wrote:

I understand. By the way, how to correctly run main part separately?

Just use run_regression.sh main-regression

Thank you.

Here is the result. prepare_tests are failed all 3. Other tests were not running.

  
   5 SEND-TEXT                                                             
value = ʼsymanʼ; modifiers = ʼʼ;                                           
delay = ʼ0ʼ; special = ʼVK_ENTERʼ;                                         

   PASSED 0.252                                                            

   6 SEND-TEXT                                                             
value = ʼtest123ʼ; modifiers = ʼʼ;                                         
delay = ʼ0ʼ; special = ʼVK_ENTERʼ;                                         

   PASSED 0.352                                                            

   7 CHECK-SCREEN-BUFFER                                                   
wait = true; millis = ʼ180000ʼ; failing screen =                           
      MMMMMM           MMMMMM         ##############                       
      MMMMMMM         MMMMMMM             ###    ## ###    ### ####   #### 
confidential info redacted
      MMMMMM           MMMMMM AAAAA   AAAAA  JJJJJJJ    IIIIIIII   CCCCCCC 
      Copyright ................
                     User Name: n                                          
                      Password:                                            
                User NOT Authorized to enter the MAJIC System!             
      Report any problems to the GSO IT Help Desk at extension: 3062.      
Please Login to the MAJIC System.                                          

   FAILED 180.095                                                          
timeout before the specific screen buffer became available (Mismatched data
lative X 1) and found ' ' (0x0000 at relative Y 0, relative X 1), template:

Can you manually start the MAJIC client, login and use the application?

Greg Shah wrote:

Can you manually start the MAJIC client, login and use the application?

Yes, It works manually.

Somehow, your test environment is completely invalid. Perhaps you are not properly following the instructions from timco.html?

When was the last time you ran this? Your changes should not be causing this failure. This is a failure to setup or run the testing environment. You need to look carefully at those instructions and figure out how you are going wrong.

Greg Shah wrote:

Somehow, your test environment is completely invalid. Perhaps you are not properly following the instructions from timco.html?

When was the last time you ran this? Your changes should not be causing this failure. This is a failure to setup or run the testing environment. You need to look carefully at those instructions and figure out how you are going wrong.

I have some 2-years old file. Could you recall where the actual timco.html file located?

~/shared/dev_tools/std_client/handbook_20160303.zip

I'd tested again and result is the same..

No one else has time to fix this for you. You need to dig into the "Unexpected throwable" issue that is appearing in your server.log. That is most likely the problem. If it isn't the only problem it is at least a real issue that is due to:

1. Your changes in 2966a. OR
2. Something invalid about how you are running testing.

I've tested the trunk and there is no such errors, so the reason in my changes. I've also rebased branch with current trunk revision. Now current revision is 11150.

Is there some relation between log file in home directory (for example client_vig_13558.log) and the test caused an exception from this log file? I.e. is it possible having log file to find out concrete failing test?

Vadim Gindin wrote:

Is there some relation between log file in home directory (for example client_vig_13558.log) and the test caused an exception from this log file? I.e. is it possible having log file to find out concrete failing test?

It is possible if you change the log output format to include time stamps. See https://docs.oracle.com/javase/7/docs/api/java/util/logging/SimpleFormatter.html for example.

I wrote more about log-file name relation to test. Does the one file corresponds to one test?

If yes, it would be more convenient to add test name info to log-file name (for example client_vig_gso_14.log. If no - the only way is to add test name to log-file content using some formatter as you advised.

I'd propose that one log file though is used for one test in most cases. Look at login_to_main_menu.xml:

..
   <send-text special="VK_ENTER">
      <substitution name="value" spec="%s%sclient/client.sh -i%d" >
         <parameter variable="buildPath" />
         <parameter variable="runDir" />
         <parameter variable="instanceNum" />
      </substitution>
   </send-text>
..

Beside previous question look at client.sh:

..
if [ $log"." == "." ]; then
   if [ $logdir"." == "." ]; then
      logdir="." 
   fi
   log="${logdir}/client_$(whoami)_${$}.log" 
fi
..

As a result we have for example "client_vig_13558.log". My next question is more about bash: ${$} how this construction works to output "13558"? Is it a special construction or just a template?

Vadim Gindin wrote:

I wrote more about log-file name relation to test. Does the one file corresponds to one test?

If yes, it would be more convenient to add test name info to log-file name (for example client_vig_gso_14.log. If no - the only way is to add test name to log-file content using some formatter as you advised.

Please correct me someone if I am wrong. The test harness that runs the tests and has the knowledge of the currently executing test is independent on P2J. Thus you will not see the test name in the P2J client/server log, unless you (1) make the harness and P2J log to the same logging target, (2) make test harness and P2J log time stamps with every log entry and merge the log files together. I am assuming the test harness is capable to log the test name being executed.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Vadim Gindin wrote:

I wrote more about log-file name relation to test. Does the one file corresponds to one test?

If yes, it would be more convenient to add test name info to log-file name (for example client_vig_gso_14.log. If no - the only way is to add test name to log-file content using some formatter as you advised.

Please correct me someone if I am wrong. The test harness that runs the tests and has the knowledge of the currently executing test is independent on P2J. Thus you will not see the test name in the P2J client/server log, unless you (1) make the harness and P2J log to the same logging target, (2) make test harness and P2J log time stamps with every log entry and merge the log files together. I am assuming the test harness is capable to log the test name being executed.

I don't think that we need any merging. I would want to add to P2J client log harness test case name..

The test harness and P2J client are two independent Java processes. P2J client has no idea that it is being driven by the test harness and not by a real human. And so P2J client also has no idea which test is currently being executed. The only way is to somehow merge the log output from the harness and P2J client.

Ok, thanks. I think I won't do logs merging.

As a result we have for example "client_vig_13558.log". My next question is more about bash: ${$} how this construction works to output "13558"? Is it a special construction or just a template?

The 13558 is the process id (pid) for the P2J client.

I've found that one harness test case like "gso_233" is not correspond to one client_*.log file.

Correct.

There is probably one client_*.log on one test-set from majic_regression_testing.xml.

Not correct. Each P2J client will run potentially multiple tests from a single driver thread. It is possible that a given test can exit from the P2J client and then start a new one. So the output for a test can even cross multiple P2J client logs.

Since there can be multiple driver threads per test set and or test plan, there are many client logs in this case.

In our current majic test baselines, every time we start a P2J client, we do execute a test that starts the client and does a login to get to the main menu.

The only way is to somehow merge the log output from the harness and P2J client.

I don't want to tie these together. It is already a complicated process but the independence between the harness and P2J is important.

Any P2J output from a test that is targeted to STDERR will go into the client log. This generally only comes from debugging or error output in P2J, because the 4GL has no way to target output to STDERR without shell commands.

Focus on the stack trace of the exception itself. What code is called from the server to make the failure happen? You can also put some diagnostic code in that location of the client to block (pause). Then watch for this blocked thread and when it occurs, capture a thread dump of the P2J server. That will show you the code that is being executed on the server side.

Thanks Greg. I'll try.

How to find out which thread is blocked?

I am assuming that you will add code to block the thread. That added code will be visible at the top of the stack trace in the thread dump, for the thread that is blocked.

Yes, I'd added the code that blocks the thread and rerun main regression testing. But how find at devsrv01 which thread is blocked? There are several java processes..

Did you generate a thread dump for the server? That will show the server-side threads. One of them will be calling the client entry point that is blocked.

What client API is being called by the blocked thread?

Do you have ANY clients attached to the server? I see no Conversation threads in the thread dump, so all your clients are dead right now.

In other words, you don't have a blocked client right now. So the thread dump cannot help. Make sure your diagnostic code is stopping a client and then get the thread dump from the server.

I'd added that code right before target Exception is thrown: AbstractWidget.topLevelWindow(). There is an Exception "Widget ... is not attached to a TopLevelWindow instance.". Regression testing is working long (may be 6 hrs). It is still running. Even if there is no block yet, there should be some clients. Strange.

After this run is finished I found that an Exception with TopLevelWindow wasn't thrown at all. Only "Unexpected throwable."

It prepended by [10/03/2016 11:45:58 EDT] (ErrorManager:SEVERE) {00000003:00000026:qryhrs} ** No employee record is available. (91)
if it matters.

After this run is finished I found that an Exception with TopLevelWindow wasn't thrown at all. Only "Unexpected throwable."

You need to change your blocking code so that you match the exception that is thrown. Perhaps you need to catch the right exception and/or check the cause of the exception you caught. The "Unexpected throwable" is a bug in your changes. Catch it, log something distinctive and use that logging output to know when to take the thread dump of the P2J server.

It prepended by [10/03/2016 11:45:58 EDT] (ErrorManager:SEVERE) {00000003:00000026:qryhrs} ** No employee record is available. (91)
if it matters.

This is a normal thing that happens in 4GL code and it is usually meaningless.

Greg Shah wrote:

After this run is finished I found that an Exception with TopLevelWindow wasn't thrown at all. Only "Unexpected throwable."

You need to change your blocking code so that you match the exception that is thrown. Perhaps you need to catch the right exception and/or check the cause of the exception you caught. The "Unexpected throwable" is a bug in your changes. Catch it, log something distinctive and use that logging output to know when to take the thread dump of the P2J server.

In my last 3 runs, there were 3 different results. 1) There was "Widget .. is not attached to a TopLevelWindow instance" 2) There was "Unexpected throwable" 3) There was no exceptions because prepare_tests are failed..

I'm currently trying to catch "Unexpected throwable". It was met more frequently. Thank's

TaApprvMaint is related to "time and attendance approval". You can look in that program for some using text associated with the frame being used for the WAIT-FOR. Then search through the comparison screens in the majic_baseline/screens/ directory. This will lead you to a list of the tests that reference that screen (or a related screen with the same text).

Then you can try each of those cases individually to see which one causes the failure.

The test gso_216:

10/10/2016        T&A - CLOCK VIOLATION APPROVAL OPTIONS         04:06:52                                                                     

Crew: A1A    Start: 11/15/2007     End: 11/15/2007                                                                             

confidential information redacted

|                                                              
|                                                                        |                                                              
|                                                                        |                                                              
+------------------------------------------------------------------------+                                                              
< Save >    < Cancel >                                                                                         

java.lang.IllegalStateException: Widget  186 is not attached to a TopLevelWindow instance.
        at com.goldencode.p2j.ui.client.widget.AbstractWidget.topLevelWindow(AbstractWidget.java:321)
        at com.goldencode.p2j.ui.client.FocusManager.focusChange(FocusManager.java:1331)
        at com.goldencode.p2j.ui.chui.ThinClient.nextFocus(ThinClient.java:18836)
        at com.goldencode.p2j.ui.client.FillIn.processKeyEvent(FillIn.java:1361)
        at com.goldencode.p2j.ui.client.widget.TitledWindow.processKeyEvent(TitledWindow.java:231)
        at com.goldencode.p2j.ui.client.widget.AbstractWidget.processEvent(AbstractWidget.java:1271)
        at com.goldencode.p2j.ui.client.widget.TitledWindow.processEvent(TitledWindow.java:200)
        at com.goldencode.p2j.ui.client.TopLevelWindow.processEvent(TopLevelWindow.java:843)
        at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:15861)
        at com.goldencode.p2j.ui.chui.ThinClient.pop(ThinClient.java:14674)

Good work, Vadim, in isolating this recreate. As I mentioned in a separate email, I am seeing this exception too (in 1735g) but I don't think I've made any changes that are related. It seems this may be a problem in the trunk.

Constantin/Hynek/Sergey/Stanislav: Have any of you seen this issue?

Greg Shah wrote:

Good work, Vadim, in isolating this recreate. As I mentioned in a separate email, I am seeing this exception too (in 1735g) but I don't think I've made any changes that are related. It seems this may be a problem in the trunk.

Constantin/Hynek/Sergey/Stanislav: Have any of you seen this issue?

I've surely seen the symptom - the exception IllegalStateException: Widget is not attached to a TopLevelWindow instance, but I don't think I have seen the issue. The exception is a generic error that may be caused by many causes.

Strangely, I don't see test failures associated with this. I say this because the test failures I see are related to the standard screen differences and the detailed reports don't show any abends. But in some testing runs I see the exception logged twice (in client logs). Considering this should cause an abend and is likely the cause of the "unexpected throwable" disconnection, I would expect this to be a real problem.

Stanislav: Do you have any guidance for Vadim on where to look for the browse bug (note 255) root cause?

I tried brws_tlw_err.p locally and I see auto-return bug in GUI - I have to press Enter to get to the next line. ChUI is OK. No abends in either case.

Nice work! The proposed change looks reasonable. Please include it in 3111f.

Greg Shah wrote:

Please include it in 3111f.

Ok, will do.

The branch is rebased to the trunk revision 11126. Regression testing is running.

Unfortunately the result is similar to previous:
gso_tests 67 failed tests,
tc_tests 254 failed tests
The errors are the same: "eating" characters during authorization.

Hynek, please clarify. Is your change already in trunk or not yet?

Vadim Gindin wrote:

Hynek, please clarify. Is your change already in trunk or not yet?

Yes, the change from note 264 has been merged to trunk from task branch 3111g.

I'm running regression testing in my home dir on devsrv01. This way we can remove your testing environment and just have an independent check of the branch.

Greg Shah wrote:

I'm running regression testing in my home dir on devsrv01. This way we can remove your testing environment and just have an independent check of the branch.

OK.

The errors are the same: "eating" characters during authorization.

I have duplicated this same result. There is some problem with the branch that leads to massive issues. In my run, I had hundreds of tests fail. The login itself fails with missing characters.

Next steps:

1. Can you duplicate this behavior manually? Try just using the login screen to check this. Does it always work? Or does the failure only occur when the system is under load? If you can recreate the problem by manually testing at the login screen, then item 2 below will be a much more efficient process because you won't have to run the main automated regression testing to check each time.

2. You need to find the change 2966a that is the cause. Remove changes (all the related edits for a given fix) one at a time and re-run testing (main regression or the manual login screen testing if you can recreate using that). When the problem goes away you will know which change caused it.

Greg Shah wrote:

The errors are the same: "eating" characters during authorization.

I have duplicated this same result. There is some problem with the branch that leads to massive issues. In my run, I had hundreds of tests fail. The login itself fails with missing characters.

Next steps:

1. Can you duplicate this behavior manually? Try just using the login screen to check this. Does it always work? Or does the failure only occur when the system is under load? If you can recreate the problem by manually testing at the login screen, then item 2 below will be a much more efficient process because you won't have to run the main automated regression testing to check each time.

Unfortunately I couldn't reproduce such errors manually in all my tries.

2. You need to find the change 2966a that is the cause. Remove changes (all the related edits for a given fix) one at a time and re-run testing (main regression or the manual login screen testing if you can recreate using that). When the problem goes away you will know which change caused it.

OK.

I'd tried to some moment add changes to p2j in testcases folder (in testing environment), but during regression-testing runs all such changes were reverted. Is there a way to add such changes right in testing environments without commits/archives?

Use an update zip file like we did before using branches. That zip will be applied over the updated branch.

After I reverted WindowManager and AbstractWidget I got 4 failed test. Have a look at attached reverted diff

I don't see any real change in the AbstractWidget diff. Did you post the right diff for that?

The changes in WindowManager are certainly more serious. We can move them to another branch and test them independently to see if it is the cause.

Hynek: what do you think?

Please re-run testing to see if those 4 failures can be passed.

Greg Shah wrote:

After I reverted WindowManager and AbstractWidget I got 4 failed test. Have a look at attached reverted diff

I don't see any real change in the AbstractWidget diff. Did you post the right diff for that?

The changes in WindowManager are certainly more serious. We can move them to another branch and test them independently to see if it is the cause.

Hynek: what do you think?

The diff shows changes several months old. Vadim, is this the correct diff?

By the way, next time you post a diff file please make sure the file extension is "diff" so that Redmine can show it in its diff view.

Greg Shah wrote:

After I reverted WindowManager and AbstractWidget I got 4 failed test. Have a look at attached reverted diff

I don't see any real change in the AbstractWidget diff. Did you post the right diff for that?

Yes. Probably during several consistent commits the changes were reverted in this file, so we can just skip it.

The changes in WindowManager are certainly more serious. We can move them to another branch and test them independently to see if it is the cause.

Hynek: what do you think?

Please re-run testing to see if those 4 failures can be passed.

OK.

Hynek Cihlar wrote:

Greg Shah wrote:

After I reverted WindowManager and AbstractWidget I got 4 failed test. Have a look at attached reverted diff

I don't see any real change in the AbstractWidget diff. Did you post the right diff for that?

The changes in WindowManager are certainly more serious. We can move them to another branch and test them independently to see if it is the cause.

Hynek: what do you think

The diff shows changes several months old. Vadim, is this the correct diff?

Yes, it is correct.

By the way, next time you post a diff file please make sure the file extension is "diff" so that Redmine can show it in its diff view.

OK, I'd just mixed that up following the same purpose ).

Regression testing finished. 2 tests are failed: gso156_session2 and gso156_session1

Here is how the error looks:

   12 POST-EVENT-SEM                                                                                                                                                                                 
name = ʼgso156ʼ; reset = true;                                                                                                                                                                       

   PASSED 0.001                                                                                                                                                                                      

   13 WAIT-EVENT-SEM                                                                                                                                                                                 
name = ʼgso156ʼ; timeout = 240000                                                                                                                                                                    

   FAILED 240.000                                                                                                                                                                                    
Timeout while waiting for event semaphore to be posted.                                                                                                                                              

What can it mean?

Did these same tests fail in the previous run?

Sure, only these tests are failed in both runs.

   40 gso156_session1 GSO 156 (Session 1) - Adding SO Op will freeze if PO Notes being added at same time. FAILED CONCURRENT BACKOUT Driver 2 243.504                                                
failure in step 13: 'Timeout while waiting for event semaphore to be posted.'                                                                                                                        

   41 gso156_session2 GSO 156 (Session 2) - Adding SO Op will freeze if PO Notes being added at same time. FAILED CONCURRENT BACKOUT Driver 3 185.020                                                
failure in step 9: 'timeout before the specific screen buffer became available (Mismatched data at line 3, column 12. Expected '1' (0x0031 at relative Y 3, relative X 12) and found '2' (0x0032 at r
elative Y 3, relative X 12), template: screens/gso/gso_156/project_operation1.txt.)'                                                                                                                 

It looks like some concurrent problem. I would suppose also to find a change, causing it first, because it also could be complex to reproduce.

Anyway in the second test (gso156_session2) there is wrong output:

11/03/2016                  PROJECT OPERATIONS                    04:54:18  
confidential info redacted
│Operation: 2                       Report Group:                              │
confidential info redacted
│ How Mal Code: 000                                           RCA Status: N    │
└──────────────────────────────────────────────────────────────────────────────┘
(F)ind (N)ext (P)rev (A)dd (U)pdate (D)elete (O)utput (2)Skills (R)eturn 
Enter choice or <SPACE> for list of additional options: N
Enter data or press F4 to end.

   FAILED 180.064
timeout before the specific screen buffer became available (Mismatched data at lin
e 3, column 12. Expected '1' (0x0031 at relative Y 3, relative X 12) and found '2'
 (0x0032 at relative Y 3, relative X 12), template: screens/gso/gso_156/project_op
eration1.txt.)

The field "Operation" contains wrong value "2", but must contain "1".

Just in case I've checked it manually. There is correct value.

The GSO 156 test is one that commonly fails. If you ran it manually and it was OK, then it is fine.

There are no other tests that failed in both runs?

Hynek: please do a code review of this branch.

Vadim: is the branch prepared for review or are you still using an update zip to remove the WindowManager change?

Greg Shah wrote:

There are no other tests that failed in both runs?

Yes, there are no other tests, that failed in both runs.

Vadim: is the branch prepared for review or are you still using an update zip to remove the WindowManager change?

No, I still use update zip. Can I make all this changes: create 2966b with WindowManager change and revert it in 2966a?

Actually I would run another cycle of runtime regression testing, because the first run was only for main-part.

Code review 2966a.

ThinClient.java
Duplicate file history entry.

• Wrong id in file history entry.
• Menu.processAccelerator() javadoc states "Process accelerator event. If some MENUBAR
  sub-menu is displayed, than event must be ignored." According to the actual implementation this
  holds true only for GUI (MenuGuiImpl), Menu.processAccelerator() never ignores the event.
  Was it an intention to have a different behavior for ChUI and GUI? If so than please update the
  javadoc of Menu.processAccelerator() so that it conforms to the implementation.
• A suggestion for the future: Is my assumption correct that triggering menu/menuitem accelerator
  and triggering menu/menuitem mnemonic should yield the same actions? If so than it would be
  better to process both events in the same manner, i.e. Menu.processAccelerator() only posts
  an ItemEvent while processMnemonicEvent() does the actual job of focusing or selecting the
  menu item. Unifying both methods to post the ItemEvent would simplify and unify the logic.

MenuItem.java
Duplicate file history entry.

MenuElement
While this was not your change, I think it would make sense to rename MenuElement.getParent()
to something more descriptive to prevent logical conflict with Widget.parent(). Maybe
MenuElement.getParentMenuElement() (and possibly make the method return MenuElement)?

OverlayWindow.hide() may lead to a NullPointerException due to the call to WindowManager.activateWindow(getOwner()), getOwner() may return null.

AbstractWidget.java has an added file history entry, but no effective change.

I added a comment to TopLevelWindow and fixed a typo in WindowManager. Checked in to 2966a.

Hynek Cihlar wrote:

Code review 2966a.

Also the methods enableOSEvents() and disableOSEvents() in WindowManager are no longer used, you probably added them in with a rebase?

Vadim Gindin wrote:

After I reverted WindowManager and AbstractWidget I got 4 failed test. Have a look at attached reverted diff

Is this related to the "eating character issue"? Does it happen in ChUI only?

Hynek: how much of that original WindowManager change is still relevant?

Can I make all this changes: create 2966b with WindowManager change and revert it in 2966a?

Actually, I would prefer if Hynek would take the important part of the change and include it with his current work.

Yes, you can remove it from 2966a.

Greg Shah wrote:

Hynek: how much of that original WindowManager change is still relevant?

All the changes are relevant except the unused methods enableOSEvents() and disableOSEvents().

Greg Shah wrote:

Can I make all this changes: create 2966b with WindowManager change and revert it in 2966a?

Actually, I would prefer if Hynek would take the important part of the change and include it with his current work.

Vadim, you can revert the changes at your convenience. I have grabbed the diff and will apply the changes to 3111h or the upcoming 3111j branch.

Hynek Cihlar wrote:

Vadim Gindin wrote:

After I reverted WindowManager and AbstractWidget I got 4 failed test. Have a look at attached reverted diff

Is this related to the "eating character issue"? Does it happen in ChUI only?

Yes. Yes for CHUI automated testing. Manually this bug is not reproduced.

Hynek, I've fixed remarks and reverted WindowManager/AbstractWidget changes. Please review again. I need permission to run testing again.

P.S. I've renamed MenuElement.getParent() to MenuElement.getParentMenuContainer(). MenuElement cannot be a Menu: only MenuItem and SubMenu. This method returns Menu or SubMenu.

Vadim Gindin wrote:

Hynek, I've fixed remarks and reverted WindowManager/AbstractWidget changes. Please review again. I need permission to run testing again.

P.S. I've renamed MenuElement.getParent() to MenuElement.getParentMenuContainer(). MenuElement cannot be a Menu: only MenuItem and SubMenu. This method returns Menu or SubMenu.

Code review 2966a. The changes look good, you can rerun the regression tests.

Here are the failed tests: gso_238, gso_364, gso178_session1, gso178_session2. All of them are met first time in this run.

I've ran 238 and 364 in a single mode. 238 has PASSED and 364 has FAILED.

   37 CHECK-SCREEN-BUFFER
wait = true; millis = ʼ180000ʼ; failing screen = 
11/07/2016              EMPLOYEE TIME CARD BATCH LOAD DETAIL          08:42:10

confidential information redacted

Enter data or press F4 to end.

   FAILED 180.100

timeout before the specific screen buffer became available (Mismatched data at line
 11, column 38. Expected 'R' (0x0052 at relative Y 11, relative X 38) and found 'F'
 (0x0046 at relative Y 11, relative X 38), template: screens/gso/gso_364/job_time_p
rocess_step9.txt.)

Is it known error?

It is difficult to tell, because the state of the database has been edited by the testing.

Please just run main-regression again and see if the failures occur again.

Greg Shah wrote:

It is difficult to tell, because the state of the database has been edited by the testing.

Please just run main-regression again and see if the failures occur again.

The next regression testing is finished. Here are the failing tests: gso_190, tc_dc_slot_027, tc_dc_slot_029. As we can see: no tests, that are failed in 2 last runs. Can I commit the changes?

Yes, please merge 2966a to trunk.

There is nothing more to do in this task?

Greg Shah wrote:

Yes, please merge 2966a to trunk.

There is nothing more to do in this task?

There is no more to do in this task except extracted WindowManager change.

2966a is committed to trunk as revisino 11128.