Project

General

Profile

Bug #2966

Bug #2677: fix drawing and functional differences between P2J GUI and 4GL GUI

NullPointerException is raised when RMB is clicked to show popup menu

Added by Vadim Gindin over 5 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Vadim Gindin
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:

window.diff.txt Magnifier (1.68 KB) Vadim Gindin, 06/27/2016 05:15 AM

extra_space.png (1.79 KB) Vadim Gindin, 06/27/2016 09:23 AM

window_activation.diff Magnifier (3.21 KB) Hynek Cihlar, 07/22/2016 09:52 AM

2966_output.txt Magnifier (2.73 KB) Vadim Gindin, 07/22/2016 11:44 AM

2966.gif (811 KB) Vadim Gindin, 07/25/2016 06:13 AM

err.log Magnifier (10.3 KB) Vadim Gindin, 07/25/2016 06:13 AM

logging.diff Magnifier (928 Bytes) Hynek Cihlar, 07/25/2016 12:05 PM

err_moveToTop.log Magnifier (14.5 KB) Vadim Gindin, 07/25/2016 12:12 PM

movetotop.diff Magnifier (1.84 KB) Hynek Cihlar, 07/25/2016 12:38 PM

conversion_err.txt Magnifier (32.6 KB) Vadim Gindin, 07/27/2016 07:50 AM

mouse_trajectory.png (2.55 KB) Vadim Gindin, 08/12/2016 04:14 AM

awt_thread.txt Magnifier - the stack for WINDOW_ACTIVATED (4.77 KB) Vadim Gindin, 08/29/2016 07:38 AM

main_thread.txt Magnifier - the stack for MENU-ITEM drawing (3.18 KB) Vadim Gindin, 08/29/2016 07:38 AM

client_logs_list.txt Magnifier (920 Bytes) Vadim Gindin, 09/30/2016 06:12 AM

jstack.txt Magnifier (64.9 KB) Vadim Gindin, 10/03/2016 04:30 PM

converstation_thread.txt Magnifier (8.87 KB) Vadim Gindin, 10/06/2016 05:28 AM

brws_tlw_err.p Magnifier (2.2 KB) Vadim Gindin, 10/11/2016 02:59 AM

focusmanager_fix.diff Magnifier (1.26 KB) Hynek Cihlar, 10/12/2016 11:38 AM

2966a_err.txt Magnifier (7.56 KB) Vadim Gindin, 11/02/2016 01:19 PM

History

#1 Updated by Vadim Gindin over 5 years ago

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.

#2 Updated by Vadim Gindin almost 5 years ago

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

#3 Updated by Vadim Gindin almost 5 years ago

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)

It happen each time when I click on SUB-MENU "John". parentOrSelf(TopLevelWindow.class); for SUB-MENU "John" returns null in AbstractWidget.topLevelWindow(AbstractWidget.java:315).

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?

#4 Updated by Hynek Cihlar almost 5 years ago

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 (?).

#5 Updated by Vadim Gindin almost 5 years ago

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?

#6 Updated by Vadim Gindin almost 5 years ago

See the change.

#7 Updated by Vadim Gindin almost 5 years ago

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..?

#8 Updated by Hynek Cihlar almost 5 years ago

Vadim Gindin wrote:

See the change.

Yes, the change is correct.

#9 Updated by Hynek Cihlar almost 5 years ago

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.

#10 Updated by Vadim Gindin almost 5 years ago

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

#11 Updated by Hynek Cihlar almost 5 years ago

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.

#12 Updated by Greg Shah almost 5 years ago

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.

#13 Updated by Vadim Gindin almost 5 years ago

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

#14 Updated by Greg Shah almost 5 years ago

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?

#15 Updated by Vadim Gindin almost 5 years ago

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.

#16 Updated by Greg Shah almost 5 years ago

  • Start date deleted (01/22/2016)
  • Assignee set to Vadim Gindin
  • Status changed from New to WIP

Does this also fix the issue described in #2952-33 ?

If not, please fix that ASAP.

#17 Updated by Greg Shah almost 5 years ago

Code Review Task Branch 2966a Revision 11057

1. Please rebase.

2. Add history entries.

#18 Updated by Vadim Gindin almost 5 years ago

  • Assignee deleted (Vadim Gindin)

Greg Shah wrote:

Code Review Task Branch 2966a Revision 11057

1. Please rebase.

2. Add history entries.

Done. See revision 11059.

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

#19 Updated by Greg Shah almost 5 years ago

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?

#20 Updated by Vadim Gindin almost 5 years ago

I'll check it.

#21 Updated by Vadim Gindin almost 5 years ago

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?

#22 Updated by Hynek Cihlar almost 5 years ago

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?

#23 Updated by Vadim Gindin almost 5 years ago

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

#24 Updated by Greg Shah almost 5 years ago

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

#25 Updated by Vadim Gindin almost 5 years ago

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:
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?

#26 Updated by Vadim Gindin almost 5 years ago

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

#27 Updated by Hynek Cihlar almost 5 years ago

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.

#28 Updated by Hynek Cihlar almost 5 years ago

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.

#29 Updated by Vadim Gindin almost 5 years ago

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)

#30 Updated by Vadim Gindin almost 5 years ago

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?

#31 Updated by Hynek Cihlar almost 5 years ago

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().

#32 Updated by Vadim Gindin almost 5 years ago

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.

#33 Updated by Vadim Gindin almost 5 years ago

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;
         });
      }
      ...

By the way activated is null here.
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.

#. 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.

#34 Updated by Vadim Gindin almost 5 years ago

Small additional description. There are possible 2 ways for menus navigation:
  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.

#35 Updated by Vadim Gindin almost 5 years ago

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?

#36 Updated by Hynek Cihlar almost 5 years ago

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.

#37 Updated by Vadim Gindin almost 5 years ago

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".

#38 Updated by Hynek Cihlar almost 5 years ago

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.

#39 Updated by Vadim Gindin almost 5 years ago

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?

#40 Updated by Hynek Cihlar almost 5 years ago

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.

#41 Updated by Vadim Gindin almost 5 years ago

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.

#42 Updated by Hynek Cihlar almost 5 years ago

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?

#43 Updated by Vadim Gindin almost 5 years ago

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?

#44 Updated by Hynek Cihlar almost 5 years ago

Vadim Gindin wrote:

How to determine ParentSm there?

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

#45 Updated by Vadim Gindin almost 5 years ago

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.

#46 Updated by Hynek Cihlar almost 5 years ago

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()?

#47 Updated by Vadim Gindin almost 5 years ago

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()

#48 Updated by Hynek Cihlar almost 5 years ago

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.

#49 Updated by Vadim Gindin almost 5 years ago

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?

#50 Updated by Hynek Cihlar almost 5 years ago

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.

#51 Updated by Vadim Gindin almost 5 years ago

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.

#52 Updated by Vadim Gindin almost 5 years ago

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.

#53 Updated by Vadim Gindin almost 5 years ago

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

#54 Updated by Vadim Gindin almost 5 years ago

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

Hynek, have you try to debug?

Need advice how to debug it..

#55 Updated by Hynek Cihlar almost 5 years ago

I was able to replicate the problem you describe (sub-menu A is hidden when its child sub-menu is closed due to mouse transferred to A). But for me it happens very rarely, I've seen it like three times or so out of hundred of tries. Does it happen for you more regularly? What are the exact steps you do?

When it happened for me I tracked this down to Swing calling Window.windowLostFocus() with WindowEvent.getOppositeWindow() == null. This indicates (or at least we interpret it this way) that the focus was transferred out to a non-P2J window and the app logic reaction to this event is correct, it closes all the overlay windows.

Please do try the attached diff, whether you observe any changes.

#56 Updated by Vadim Gindin almost 5 years ago

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

#57 Updated by Hynek Cihlar almost 5 years ago

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

#58 Updated by Vadim Gindin almost 5 years ago

Here it is.

#59 Updated by Hynek Cihlar almost 5 years ago

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.

#60 Updated by Hynek Cihlar almost 5 years ago

Vadim Gindin wrote:

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

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

#61 Updated by Vadim Gindin almost 5 years ago

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

#62 Updated by Hynek Cihlar almost 5 years ago

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?

#63 Updated by Vadim Gindin almost 5 years ago

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.

#64 Updated by Hynek Cihlar almost 5 years ago

Vadim Gindin wrote:

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.

Please apply the attached diff, do another run (with the problem appearing) and post the log.

#65 Updated by Vadim Gindin almost 5 years ago

Here it is

#66 Updated by Hynek Cihlar almost 5 years ago

Vadim Gindin wrote:

Here it is

Please apply the attached diff and retest.

#67 Updated by Vadim Gindin almost 5 years ago

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?

#68 Updated by Hynek Cihlar almost 5 years ago

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.

#69 Updated by Vadim Gindin almost 5 years ago

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!

#70 Updated by Hynek Cihlar almost 5 years ago

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.

#71 Updated by Hynek Cihlar almost 5 years ago

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".

#72 Updated by Vadim Gindin almost 5 years ago

Should I commit the your changes and debugging code?

#73 Updated by Hynek Cihlar almost 5 years ago

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.

#74 Updated by Vadim Gindin almost 5 years ago

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

#75 Updated by Greg Shah almost 5 years ago

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.

#76 Updated by Vadim Gindin almost 5 years ago

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

#77 Updated by Greg Shah almost 5 years ago

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.

#78 Updated by Vadim Gindin almost 5 years ago

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?

#79 Updated by Hynek Cihlar almost 5 years ago

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.

#80 Updated by Vadim Gindin almost 5 years ago

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

Is it known bug?

#81 Updated by Hynek Cihlar almost 5 years ago

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?

#82 Updated by Vadim Gindin almost 5 years ago

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.

#83 Updated by Vadim Gindin almost 5 years ago

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?

#84 Updated by Hynek Cihlar almost 5 years ago

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.

#85 Updated by Vadim Gindin almost 5 years ago

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

tt is a DMO object of the interface Tt1 (dmo/_temp/Tt1.java). It contains only getters/setters for f1, f2, f3, f4.

#86 Updated by Greg Shah almost 5 years ago

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.

#87 Updated by Vadim Gindin almost 5 years ago

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]


Is the server working?

#88 Updated by Constantin Asofiei almost 5 years ago

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.

#89 Updated by Vadim Gindin almost 5 years ago

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?

#90 Updated by Vadim Gindin almost 5 years ago

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)

Probably some misconfiguration.

About simple_sm.p. I've faced with 2 bugs:
  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?

#91 Updated by Greg Shah almost 5 years ago

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.

#92 Updated by Vadim Gindin almost 5 years ago

Greg Shah wrote:

Probably some misconfiguration.

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

it is activated..

#93 Updated by Vadim Gindin almost 5 years ago

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..

#94 Updated by Vadim Gindin almost 5 years ago

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?

#95 Updated by Hynek Cihlar almost 5 years ago

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.

#96 Updated by Vadim Gindin almost 5 years ago

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.

#97 Updated by Vadim Gindin almost 5 years ago

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..

#98 Updated by Greg Shah almost 5 years ago

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.

#99 Updated by Greg Shah almost 5 years ago

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

#100 Updated by Greg Shah almost 5 years ago

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.

#101 Updated by Eric Faulhaber almost 5 years ago

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.

#102 Updated by Vadim Gindin almost 5 years ago

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.

#103 Updated by Vadim Gindin almost 5 years ago

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".

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..

#104 Updated by Vadim Gindin almost 5 years ago

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.

#105 Updated by Eric Faulhaber almost 5 years ago

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.

#106 Updated by Vadim Gindin almost 5 years ago

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.

#107 Updated by Greg Shah almost 5 years ago

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.

#108 Updated by Vadim Gindin almost 5 years ago

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?

#109 Updated by Greg Shah almost 5 years ago

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?

#110 Updated by Vadim Gindin almost 5 years ago

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.

#111 Updated by Vadim Gindin almost 5 years ago

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

#112 Updated by Greg Shah almost 5 years ago

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.

#113 Updated by Greg Shah over 4 years ago

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

#114 Updated by Vadim Gindin over 4 years ago

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

#115 Updated by Greg Shah over 4 years ago

Vadim Gindin wrote:

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

Can you finish this today?

#116 Updated by Vadim Gindin over 4 years ago

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.

#117 Updated by Vadim Gindin over 4 years ago

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"):

         Widget<GuiOutputManager> mouseTarget = 
                  topLevelWindow().findMouseSource(new NativePoint(e.getX(), e.getY()));

It should be a target sub-menu "Smooth operator", but it become SubMenuBody of the first sub-menu "SM2 Submenu". The point from the event e is the following: [99,2] and sub-menu body phisical bounds is [0,0,107,111].
Ok, I'd added mouseExited for the SubMenuBody, but it wasn't called..

#118 Updated by Hynek Cihlar over 4 years ago

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().

#119 Updated by Vadim Gindin over 4 years ago

#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?

#120 Updated by Hynek Cihlar over 4 years ago

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.

#121 Updated by Vadim Gindin over 4 years ago

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..

#122 Updated by Hynek Cihlar over 4 years ago

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.

#123 Updated by Vadim Gindin over 4 years ago

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.

#124 Updated by Hynek Cihlar over 4 years ago

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?

#125 Updated by Vadim Gindin over 4 years ago

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?

Yes, it can probably. It ("pressed" flag in MenuGuiImpl called mbarPressedState) can work. Please see the latest revision (11096), but there is one remained problem. We need to reset this flag in a right place. Lets consider 2 scenarios.
  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.

#126 Updated by Hynek Cihlar over 4 years ago

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.

#127 Updated by Hynek Cihlar over 4 years ago

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.

#128 Updated by Vadim Gindin over 4 years ago

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.

#129 Updated by Hynek Cihlar over 4 years ago

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.

#130 Updated by Vadim Gindin over 4 years ago

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.

#131 Updated by Hynek Cihlar over 4 years ago

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.

#132 Updated by Vadim Gindin over 4 years ago

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..

#133 Updated by Hynek Cihlar over 4 years ago

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?

#134 Updated by Vadim Gindin over 4 years ago

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.

#135 Updated by Vadim Gindin over 4 years ago

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.

#136 Updated by Hynek Cihlar over 4 years ago

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.

#137 Updated by Vadim Gindin over 4 years ago

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.

#138 Updated by Vadim Gindin over 4 years ago

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)

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

#139 Updated by Hynek Cihlar over 4 years ago

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.

#140 Updated by Vadim Gindin over 4 years ago

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).

#141 Updated by Hynek Cihlar over 4 years ago

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.

#142 Updated by Hynek Cihlar over 4 years ago

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.

#143 Updated by Vadim Gindin over 4 years ago

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

#144 Updated by Hynek Cihlar over 4 years ago

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?

#145 Updated by Vadim Gindin over 4 years ago

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.

#146 Updated by Hynek Cihlar over 4 years ago

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.

#147 Updated by Vadim Gindin over 4 years ago

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.

#148 Updated by Hynek Cihlar over 4 years ago

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.

#149 Updated by Hynek Cihlar over 4 years ago

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.

#150 Updated by Vadim Gindin over 4 years ago

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.

#151 Updated by Hynek Cihlar over 4 years ago

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.

#152 Updated by Vadim Gindin over 4 years ago

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.

#153 Updated by Hynek Cihlar over 4 years ago

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.

#154 Updated by Vadim Gindin over 4 years ago

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.

#155 Updated by Greg Shah over 4 years ago

Is there anything left to work in this task?

#156 Updated by Vadim Gindin over 4 years ago

Yes. Key navigation is not fully fixed.

#157 Updated by Greg Shah over 4 years ago

How quickly can this be done?

#158 Updated by Vadim Gindin over 4 years ago

Greg Shah wrote:

How quickly can this be done?

I hope I'll finish it tomorrow

#159 Updated by Greg Shah over 4 years ago

How quickly can this be done?

I hope I'll finish it tomorrow

OK.

#160 Updated by Vadim Gindin over 4 years ago

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;
      }

Here setInitialFocus() call set the first item of sub-menu as focused in the following order using parent() to get the parent widget:
submenu_body.focus = menu_item["Mi&2 item"].
bordered_panel.focus = submenu_body.
overlay_window.focus = bordered_panel.

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:

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

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.

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?

#161 Updated by Hynek Cihlar over 4 years ago

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.

#162 Updated by Vadim Gindin over 4 years ago

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.

#163 Updated by Hynek Cihlar over 4 years ago

Vadim Gindin wrote:

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

And do you see the same behavior?

#164 Updated by Vadim Gindin over 4 years ago

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.

#165 Updated by Vadim Gindin over 4 years ago

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?

#166 Updated by Vadim Gindin over 4 years ago

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?

#167 Updated by Hynek Cihlar over 4 years ago

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.

#168 Updated by Hynek Cihlar over 4 years ago

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)

#169 Updated by Vadim Gindin over 4 years ago

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)

#170 Updated by Vadim Gindin over 4 years ago

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..

#171 Updated by Hynek Cihlar over 4 years ago

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.

#172 Updated by Hynek Cihlar over 4 years ago

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.

#173 Updated by Vadim Gindin over 4 years ago

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?

#174 Updated by Vadim Gindin over 4 years ago

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.

#175 Updated by Hynek Cihlar over 4 years ago

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.

#176 Updated by Vadim Gindin over 4 years ago

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.

#177 Updated by Hynek Cihlar over 4 years ago

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?

#178 Updated by Vadim Gindin over 4 years ago

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.

#179 Updated by Hynek Cihlar over 4 years ago

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.

#180 Updated by Vadim Gindin over 4 years ago

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.

#181 Updated by Vadim Gindin over 4 years ago

It is 2 separate threads:

Thread [main] (Suspended (breakpoint at line 512 in MenuItemGuiImpl$1))    
        MenuItemGuiImpl$1.drawTitleRectangle(NativeDimension) line: 512    
        ..

and
Thread [AWT-EventQueue-0] (Suspended (breakpoint at line 1353 in WindowManager))    
    WindowManager.windowActivated(int, boolean) line: 1353    
        ..

Full stacks are in attached files.

#182 Updated by Hynek Cihlar over 4 years ago

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.

#183 Updated by Vadim Gindin over 4 years ago

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()?

#184 Updated by Hynek Cihlar over 4 years ago

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.

#185 Updated by Greg Shah over 4 years ago

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?

#186 Updated by Hynek Cihlar over 4 years ago

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.

#187 Updated by Greg Shah over 4 years ago

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.

#188 Updated by Vadim Gindin over 4 years ago

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.

#189 Updated by Hynek Cihlar over 4 years ago

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.

#190 Updated by Hynek Cihlar over 4 years ago

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.

#191 Updated by Hynek Cihlar over 4 years ago

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.

#192 Updated by Vadim Gindin over 4 years ago

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.

#193 Updated by Vadim Gindin over 4 years ago

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..

#194 Updated by Greg Shah over 4 years ago

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?

#195 Updated by Vadim Gindin over 4 years ago

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.

#196 Updated by Hynek Cihlar over 4 years ago

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.

#197 Updated by Greg Shah over 4 years ago

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.

#198 Updated by Vadim Gindin over 4 years ago

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.

#199 Updated by Vadim Gindin over 4 years ago

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

#200 Updated by Hynek Cihlar over 4 years ago

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.

#201 Updated by Vadim Gindin over 4 years ago

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?

#202 Updated by Hynek Cihlar over 4 years ago

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.

#203 Updated by Vadim Gindin over 4 years ago

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.

#204 Updated by Hynek Cihlar over 4 years ago

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.

#205 Updated by Vadim Gindin over 4 years ago

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.

#206 Updated by Vadim Gindin over 4 years ago

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.)    

#207 Updated by Constantin Asofiei over 4 years ago

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.

#208 Updated by Vadim Gindin over 4 years ago

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.

But I've found:
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)

#209 Updated by Constantin Asofiei over 4 years ago

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.

#210 Updated by Hynek Cihlar over 4 years ago

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.

#211 Updated by Vadim Gindin over 4 years ago

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 '

Logs contain a lot of following errors (server_gso_0.log):
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)

or (server.log)
[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)

#212 Updated by Hynek Cihlar over 4 years ago

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.

#213 Updated by Vadim Gindin over 4 years ago

Client logs do not contain exceptions

#214 Updated by Greg Shah over 4 years ago

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

#215 Updated by Vadim Gindin over 4 years ago

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

Should I run chmod for these folders? Are there some changes in environment setting process?

#216 Updated by Constantin Asofiei over 4 years ago

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).

#217 Updated by Vadim Gindin over 4 years ago

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

Is that way correct?

#218 Updated by Constantin Asofiei over 4 years ago

Vadim Gindin wrote:

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

Just use run_regression.sh main-regression

#219 Updated by Vadim Gindin over 4 years ago

Thank you.

#220 Updated by Vadim Gindin over 4 years ago

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:

#221 Updated by Greg Shah over 4 years ago

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

#222 Updated by Vadim Gindin over 4 years ago

Greg Shah wrote:

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

Yes, It works manually.

#223 Updated by Greg Shah over 4 years ago

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.

#224 Updated by Vadim Gindin over 4 years ago

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?

#225 Updated by Greg Shah over 4 years ago

~/shared/dev_tools/std_client/handbook_20160303.zip

#226 Updated by Vadim Gindin over 4 years ago

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

#227 Updated by Greg Shah over 4 years ago

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.

#228 Updated by Vadim Gindin over 4 years ago

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.

#229 Updated by Vadim Gindin over 4 years ago

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?

#230 Updated by Hynek Cihlar over 4 years ago

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.

#231 Updated by Vadim Gindin over 4 years ago

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.

#232 Updated by Vadim Gindin over 4 years ago

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>
..

What do you think?

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?

#233 Updated by Hynek Cihlar over 4 years ago

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.

#234 Updated by Vadim Gindin over 4 years ago

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..

I've found that one harness test case like "gso_233" is not correspond to one client_*.log file. There is probably one client_*.log on one test-set from majic_regression_testing.xml. So the only way is to add the harness test name to client_ file content if it possible. But how to get it from P2J...

#235 Updated by Hynek Cihlar over 4 years ago

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.

#236 Updated by Vadim Gindin over 4 years ago

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

#237 Updated by Greg Shah over 4 years ago

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.

#238 Updated by Vadim Gindin over 4 years ago

Thanks Greg. I'll try.

#239 Updated by Vadim Gindin over 4 years ago

How to find out which thread is blocked?

#240 Updated by Greg Shah over 4 years ago

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.

#241 Updated by Vadim Gindin over 4 years ago

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..

#242 Updated by Greg Shah over 4 years ago

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.

#243 Updated by Vadim Gindin over 4 years ago

Here it is:

There is one blocked thread, but I don't see relation to client code..

#244 Updated by Greg Shah over 4 years ago

What client API is being called by the blocked thread?

#245 Updated by Greg Shah over 4 years ago

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.

#246 Updated by Vadim Gindin over 4 years ago

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.

#247 Updated by Vadim Gindin over 4 years ago

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

#248 Updated by Vadim Gindin over 4 years ago

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

#249 Updated by Greg Shah over 4 years ago

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.

#250 Updated by Greg Shah over 4 years ago

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.

#251 Updated by Vadim Gindin over 4 years ago

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

#252 Updated by Vadim Gindin over 4 years ago

I've finally catch blocking in AbstractWidget.topLevelWindow(). Here is the Converstation thread stack attached. There is a aero.timco.majic.dc.TaApprvMaint test noted.

#253 Updated by Greg Shah over 4 years ago

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.

#254 Updated by Vadim Gindin over 4 years ago

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 >                                                                                         

When the screen "T&A - CLOCK VIOLATION APPROVAL OPTIONS" is opened first time and I press "n" this error happen:
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)

But this error happen not each time and somehow related to servers restart.

#255 Updated by Vadim Gindin over 4 years ago

Here is the test that constantly reproduces this error:

1. This error also exists in trunk (locally).. Strange, because I'd ran full regression testing of trunk at some moment and there were no such error in logs.

2. There is a browse with editable FillIn's in the last column (logical Y/N). When I press 'N' the current FillIn got new value "N" and the focused row is changed down to the next. During that new FillIn is created for new current row. The error happen for old FillIn (from previous row), because its parent became null at some moment.

┌─────── Clock Violations ────────┐ 
│   EmpNo Employee Name      Y/N  │ 
│  ─────────────────────────────  │ 
│       0 Sdfsdfasdfasd      N    │ 
│ >     1 Sdfsdfasdfasd      Y    │ 

#256 Updated by Greg Shah over 4 years ago

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?

#257 Updated by Hynek Cihlar over 4 years ago

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.

#258 Updated by Greg Shah over 4 years ago

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?

#259 Updated by Stanislav Lomany over 4 years ago

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.

#260 Updated by Hynek Cihlar over 4 years ago

  • File focusmanager_fix.diff added

I can reproduce the error java.lang.IllegalStateException: Widget 29 is not attached to a TopLevelWindow instance. when running brws_tlw_err.p.

I think the problem is in FocusManager.focusChange(). See the following stack trace showing the point at which the fill-in widget is destroyed on key press.

com.goldencode.util.LogHelper.trace(LogHelper.java:252)
com.goldencode.p2j.ui.client.widget.AbstractWidget.destroy(AbstractWidget.java:1502)
com.goldencode.p2j.ui.client.widget.AbstractContainer.destroy(AbstractContainer.java:981)
com.goldencode.p2j.ui.client.LabeledDataContainer.destroy(LabeledDataContainer.java:326)
com.goldencode.p2j.ui.client.widget.AbstractContainer.remove(AbstractContainer.java:927)
com.goldencode.p2j.ui.client.Browse.stopEdit(Browse.java:2576)
com.goldencode.p2j.ui.client.Browse.releaseCurrentRow(Browse.java:3136)
com.goldencode.p2j.ui.client.Browse.releaseCurrentRow(Browse.java:3117)
com.goldencode.p2j.ui.client.Browse.leaveRow(Browse.java:5703)
com.goldencode.p2j.ui.client.Browse.goDown(Browse.java:5058)
com.goldencode.p2j.ui.client.Browse.goDown(Browse.java:5041)
com.goldencode.p2j.ui.client.Browse.nextFocus(Browse.java:1707)
com.goldencode.p2j.ui.client.FocusManager.focusChange(FocusManager.java:1311)
com.goldencode.p2j.ui.chui.ThinClient.nextFocus(ThinClient.java:18822)
com.goldencode.p2j.ui.client.FillIn.processKeyEvent(FillIn.java:1361)
com.goldencode.p2j.ui.client.widget.TitledWindow.processKeyEvent(TitledWindow.java:231)

The interesting frame from the callstack above is com.goldencode.p2j.ui.client.FocusManager.focusChange(FocusManager.java:1311). At this point, focus manager temporarily changes focus to the next widget and browse reacts by destroying the passed in widget (old focus). It looks like the current logic doesn't expect the widget (old focus) to be destroyed after the temporary change while the intention was different as the code comment a few lines later in FocusManager.focusChange() suggests: for browse editing fill-in, the oldFocus fill-in gets destroyed during temporary focus change by Browse.goDown.

IMHO the solution is to expect the focused widget may get destroyed after the temporary focus change, see the attached diff.

#261 Updated by Hynek Cihlar over 4 years ago

Improved comment in focusmanager_fix.diff.

#262 Updated by Hynek Cihlar over 4 years ago

  • File deleted (focusmanager_fix.diff)

#263 Updated by Greg Shah over 4 years ago

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

#264 Updated by Hynek Cihlar over 4 years ago

Greg Shah wrote:

Please include it in 3111f.

Ok, will do.

#265 Updated by Vadim Gindin over 4 years ago

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

#266 Updated by Vadim Gindin over 4 years ago

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.

#267 Updated by Vadim Gindin over 4 years ago

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

#268 Updated by Hynek Cihlar over 4 years ago

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.

#269 Updated by Greg Shah over 4 years ago

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.

#270 Updated by Vadim Gindin over 4 years ago

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.

#271 Updated by Greg Shah over 4 years ago

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.

#272 Updated by Vadim Gindin over 4 years ago

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.

#273 Updated by Vadim Gindin over 4 years ago

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?

#274 Updated by Greg Shah over 4 years ago

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

#275 Updated by Vadim Gindin over 4 years ago

Greg Shah wrote:

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

#276 Updated by Greg Shah over 4 years ago

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.

#277 Updated by Hynek Cihlar over 4 years ago

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.

#278 Updated by Vadim Gindin over 4 years ago

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.

#279 Updated by Vadim Gindin over 4 years ago

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 ).

#280 Updated by Vadim Gindin over 4 years ago

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

#281 Updated by Vadim Gindin over 4 years ago

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?

#282 Updated by Greg Shah over 4 years ago

Did these same tests fail in the previous run?

#283 Updated by Vadim Gindin over 4 years ago

Sure, only these tests are failed in both runs.

#284 Updated by Vadim Gindin over 4 years ago

   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.

#285 Updated by Vadim Gindin over 4 years ago

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.

#286 Updated by Greg Shah over 4 years ago

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?

#287 Updated by Greg Shah over 4 years ago

Hynek: please do a code review of this branch.

#288 Updated by Greg Shah over 4 years ago

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

#289 Updated by Vadim Gindin over 4 years ago

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?

#290 Updated by Vadim Gindin over 4 years ago

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

#291 Updated by Hynek Cihlar over 4 years ago

Code review 2966a.

ThinClient.java
Duplicate file history entry.

Menu.java
  • 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.

#292 Updated by Hynek Cihlar over 4 years ago

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?

#293 Updated by Hynek Cihlar over 4 years ago

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?

#294 Updated by Greg Shah over 4 years ago

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

#295 Updated by Greg Shah over 4 years ago

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.

#296 Updated by Hynek Cihlar over 4 years ago

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().

#297 Updated by Hynek Cihlar over 4 years ago

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.

#298 Updated by Vadim Gindin over 4 years ago

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.

#299 Updated by Vadim Gindin over 4 years ago

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.

#300 Updated by Hynek Cihlar over 4 years ago

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.

#301 Updated by Vadim Gindin over 4 years ago

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

#302 Updated by Vadim Gindin over 4 years ago

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?

#303 Updated by Greg Shah over 4 years ago

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.

#304 Updated by Vadim Gindin over 4 years ago

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?

#305 Updated by Greg Shah over 4 years ago

Yes, please merge 2966a to trunk.

There is nothing more to do in this task?

#306 Updated by Vadim Gindin over 4 years ago

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.

#307 Updated by Greg Shah over 4 years ago

  • Target version set to Milestone 16
  • % Done changed from 0 to 100
  • Status changed from WIP to Closed
  • Assignee set to Vadim Gindin

#308 Updated by Greg Shah over 4 years ago

  • Target version changed from Milestone 16 to Cleanup and Stabilization for GUI

Also available in: Atom PDF