FWD » Core Development » User Interface

Using ./toggle_box/gui/tbx_present.p, Vadim Gindin reported:

"Right mouse button press is fully ignored in PROGRESS, but in P2J it moves the focus to pressed toggle-box and highlight checkbox at a time of pressing."

It seems, right click/press event on some widget (TOGGLE-BOX, FILL-IN or all other) does not raise ENTRY event and does not lead to focus request.

Please post a diff text file (use something like bzr diff -cREVISION > proposed_change.txt) that shows your proposed changes. Otherwise, I cannot review your work.

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim: Even if everyone is OK with the change, it cannot be checked in to 2677a right now, since the branch is locked.

Greg Shah wrote:

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim, I think that a sensitive widget should still receive focus when it is right clicked (also middle clicked?) (even if its context menu is shown). But the code change in ThinClient will only allow left-click, right?

Hynek Cihlar wrote:

Greg Shah wrote:

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim, I think that a sensitive widget should still receive focus when it is right clicked (also middle clicked?) (even if its context menu is shown). But the code change in ThinClient will only allow left-click, right?

Hynek, I also thought so, but I found a few days ago, that it is wrong and only the left-click leads to focus request and ENTRY sending. If a target widget allows context menu, than ENTRY event is sent and a focus is requested only after context menu will be closed. At least it works in a such a way for the following test:

def var cb as logical view-as toggle-box.
def var ch1 as character.
def var ch2 as character.

def var rb as integer initial 1 
   view-as radio-set 
   radio-buttons "--1--", 1, "--2--", 2, "--3--", 3.

def var comb1 as character
    view-as combo-box 
    list-items "Combo1", "Combo2", "Combo3".

def button btn-exit label "Exit".

def frame frame1 ch1 skip(2) ch2 skip(2) cb skip(2) rb skip(2) comb1 skip(2)  btn-exit with side-labels centered.
frame frame1:height-pixels = 450.
frame frame1:width-pixels = 360.

enable all with frame frame1.

on entry of cb 
do:
   message "ENTRY cb". 
end.

on entry of ch1 
do:
   message "ENTRY ch1". 
end.

on entry of ch2 
do:
   message "ENTRY ch2". 
end.

on entry of rb 
do:
   message "ENTRY rb". 
end.

on entry of comb1 
do:
   message "ENTRY comb1". 
end.

wait-for choose of btn-exit.

More over, it doesn't matter where you are clicking (right or middle click) on: widget title, checkbox (for checbox or radio) or option title for combobox. Such clicks are ignored.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Greg Shah wrote:

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim, I think that a sensitive widget should still receive focus when it is right clicked (also middle clicked?) (even if its context menu is shown). But the code change in ThinClient will only allow left-click, right?

Hynek, I also thought so, but I found a few days ago, that it is wrong and only the left-click leads to focus request and ENTRY sending. If a target widget allows context menu, than ENTRY event is sent and a focus is requested only after context menu will be closed. At least it works in a such a way for the following test:
[...]

Indeed many of the widgets do behave like this. However, some widgets in 4GL still grab the focus when right-clicked. For example FILL-IN or EDITOR.

Hynek Cihlar wrote:

Vadim Gindin wrote:

Hynek Cihlar wrote:

Greg Shah wrote:

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim, I think that a sensitive widget should still receive focus when it is right clicked (also middle clicked?) (even if its context menu is shown). But the code change in ThinClient will only allow left-click, right?

Hynek, I also thought so, but I found a few days ago, that it is wrong and only the left-click leads to focus request and ENTRY sending. If a target widget allows context menu, than ENTRY event is sent and a focus is requested only after context menu will be closed. At least it works in a such a way for the following test:
[...]

Indeed many of the widgets do behave like this. However, some widgets in 4GL still grab the focus when right-clicked. For example FILL-IN or EDITOR.

Hynek, I've checked it again. FillIn and Editor gets focus in sense of they gets a cursor blinked in corresponding field, but I meant that when widget gets a focus it's title is drawn inside dotted rectangle.

My change resides in ThinClient.processProgressEvent(..) line 15264:

            maySwitchFocus = (mouseSource != null && 
                              evtID == MouseEvt.MOUSE_PRESSED && 
                              evt.getEvent().getButton() == MouseEvent.BUTTON1);

1. Leave the current change, that add right mouse button ignoring for all widgets and fix FillIn and Editor cases later.
2. Add widget instance check for some of widgets: toggle-box, combo-box, radio-set, button or just for my toggle-box instead of current change.
3. Add some method like isRMBFocusSupported() to Widget interface.

What is more preferable in current situation?

but displays blinked cursor later when pop-up menu will be closed

Vadim, have you tested without a popup menu registered? I suspect that the behavior will be different in that case.

Also: doesn't the right mouse click still activate the window if some other window was previously active?

Greg Shah wrote:

but displays blinked cursor later when pop-up menu will be closed

Vadim, have you tested without a popup menu registered? I suspect that the behavior will be different in that case.

Greg, that was system pop-up menu, you know, standard menu with CUT/COPY/PASTE and other operations.

Also: doesn't the right mouse click still activate the window if some other window was previously active?

Yes, I've created 2 empty windows and right mouse click activates inactive window that was clicked.

Vadim Gindin wrote:

..
There are possible 3 variants:

1. Leave the current change, that add right mouse button ignoring for all widgets and fix FillIn and Editor cases later.
2. Add widget instance check for some of widgets: toggle-box, combo-box, radio-set, button or just for my toggle-box instead of current change.
3. Add some method like isRMBFocusSupported() to Widget interface.

What is more preferable in current situation?

So what we decide?

Before we go further with this, I need to address some questions. Some screen captures might help too.

1. You are discussing the default popup menu for a FILL-IN. Please show a screen capture of the menu.
2. This default menu exists for all FILL-INs in GUI?
3. What other widgets also show this default menu? Please check ALL widgets including images, buttons, frames and so forth.
4. Does the default menu appear for the WINDOW too? Check the different parts like the border, titlebar, message area, status area, system icon and min/max/close buttons.
5. For the widgets that do support the default menu, is the menu always the same?
6. For the widgets that do support the default menu, is there any way to disable it?
7. For the widgets that do support the default menu, I assume it doesn't exist if you have a 4GL popup menu associated with that widget. Correct?

We have no such support for the above right now. Before we start ignoring right mouse clicks, we first need to understand this missing functionality (and probably add it).

This will also help us understand the extent of right mouse behavior across the widget types, which is needed before we do something like globally disable right mouse processing.

Currently I'd checked Toggle-box, FillIn, Editor, Slider, Selection-list, Combobox, Button.

Does it show in Toggle-box, Slider, Selection-list, Combobox, or Button?

Greg Shah wrote:

..
Are there any other items that would need to be implemented or fixed for this task?

If we assume that right mouse button "ignoring" will be implemented as part of ENTRY proper handling (4 and 5 points), that there are no other items here except described above.

Vadim Gindin wrote:

Greg Shah wrote:

I'm changing the name of this task to more accurately reflect the work needed.

1. For the system icon and titlebar, please extend the popup menu support to the rest of the titlebar. If there are any visual deviations in the system popup menu, please fix them. Consult with Constantin on any questions about how to integrate the menu into the current implementation.

This popup menu is already works for title bar, but there are 3 issues:

1. If I click using RMB on one of system icons (minimize/maximize/close) it calls corresponding action as if I would click using left mouse button. RMB must be ignored.

1. System icons (minimize/maximize/close) have corresponding tooltips.

I made a misprint. Read "buttons" instead "icons" in this issue.

1. Items on popup sub-menu have icons.
   ...

I've created a branch 2849a and committed caption buttons tooltip implementation there. rev #10962

Icons for menu-items are not implemented for menus. MENU-ITEM in usual menus (popup or menubar) does not support icon, so will it be correct to add that support to MenuItemGuiImpl or it would be better to create sub-class especially for system popup menus? What priority of that?

Constantin, what can you reccomend about implementation of system popup menus for FillIn or Editor widgets?

As I can see WindowGuiImpl class contains implementation of popup menu in createPopup method, that creates menu dynamically. I would propose to do the same for FillIn and Editor widgets with additional check WidgetConfig.popupMenuId == -1, i.e. when popup menu is not assigned manually. But there is some difference: system popup is activated only for click in the editing field, but manual popup can be activated when the user clicks on widget label (I'll check it).

Vadim Gindin wrote:

Constantin, what can you reccomend about implementation of system popup menus for FillIn or Editor widgets?

I think a subclass of MenuItemGuiImpl is better, to add support for the left-side icon.

As I can see WindowGuiImpl class contains implementation of popup menu in createPopup method, that creates menu dynamically. I would propose to do the same for FillIn and Editor widgets with additional check WidgetConfig.popupMenuId -1, i.e. when popup menu is not assigned manually. But there is some difference: system popup is activated only for click in the editing field, but manual popup can be activated when the user clicks on widget label (I'll check it).

1. is the system popup for FILL-IN and EDITOR the same? If so, only one implementation is required
2. do not build the system popup for these more than once - cache it somewhere and re-use that, if WidgetConfig.popupMenuId -1.

OK. I need to check if popup menu is assigned manually at the moment of click. So what approach will be more preferable: register MenuPopupable action as it happen for WindowTitleBar or straight call, showing the menu?

Constantin Asofiei wrote:

..
Yes, we need a similar approach to the title-bar popup. But consider this:

1. is the system popup for FILL-IN and EDITOR the same? If so, only one implementation is required

Yes, it is the same for FILL-IN, EDITOR, TEXT widgets. It is also depends on selection. If some text is selected in a widget, CUT/COPY,PASTE,DELETE items will be enabled, but SELECT ALL will be disabled. When no text is selected COPY/CUT/DELETE items will be disabled. But this behavior is also the same for these widgets.

OK, I've added Menu editorPopup field to WindowGuiImpl and I'm going to init and reuse it each time.

..

Vadim Gindin wrote:

OK. I need to check if popup menu is assigned manually at the moment of click. So what approach will be more preferable: register MenuPopupable action as it happen for WindowTitleBar or straight call, showing the menu?

Use the same approach as for the custom menu for a widget, for now (i.e. if not a custom menu, show the standard menu, using the current selection, enabled/disabled widget, etc, to disable/enable menu items for the OS popup menu). Registering a MousePopupable for all widgets I don't think is OK at this time.

Vadim Gindin wrote:

OK, I've added Menu editorPopup field to WindowGuiImpl and I'm going to init and reuse it each time.

I think this is good, but ensure that when you are re-using it, the items are enabled/disabled/etc depending on selection. Also, check what happens if the widget is disabled or read-only. And also make sure the OS popup can be shown via keys (if 4GL allows this).

1. Should I add implementations for clipboard menu-items? Is there some Clipboard implementation?
2. Should I also add implementations for scroll operations?

Should I add implementations for clipboard menu-items?

Yes, however we know that the web client used in some browsers will have a problem with the implementation. Still, you should go ahead with it.

Is there some Clipboard implementation?

The CLIPBOARD system handle is not supported yet and the current GUI project does not require it.

But we do support CTRL-C (copy), CTRL-X (cut), and CTRL-V (paste) keyboard shortcuts. So you can "tie into" those implementations. It may make sense to just generate the key events for these.

Should I also add implementations for scroll operations?

Yes. Again, I think the easiest way is to generate the matching key events.

Does somebody remember how is the text selection works for FillInGuiImpl widget? I'm trying to find out what I should do to implement "Select All" trigger for corresponding menu item in default popup menu of FillIn. I could only try to set config.selectionStart and config.selectionEnd variables, but it was not sufficient..
EditorGuiImpl contain setSelectionText() method, but there is a little different implementation. Need help

Vadim Gindin wrote:

Does somebody remember how is the text selection works for FillInGuiImpl widget? I'm trying to find out what I should do to implement "Select All" trigger for corresponding menu item in default popup menu of FillIn. I could only try to set config.selectionStart and config.selectionEnd variables, but it was not sufficient..
EditorGuiImpl contain setSelectionText() method, but there is a little different implementation. Need help

Setting the selection start/end vars should be OK for FILL-IN; are you doing this in an event drawing bracket? Have you called repaint for the fill-in, after setting the vars?

Constantin Asofiei wrote:

Vadim Gindin wrote:

Does somebody remember how is the text selection works for FillInGuiImpl widget? I'm trying to find out what I should do to implement "Select All" trigger for corresponding menu item in default popup menu of FillIn. I could only try to set config.selectionStart and config.selectionEnd variables, but it was not sufficient..
EditorGuiImpl contain setSelectionText() method, but there is a little different implementation. Need help

Setting the selection start/end vars should be OK for FILL-IN; are you doing this in an event drawing bracket? Have you called repaint for the fill-in, after setting the vars?

Yes, I tried that, but it didn't helped.

It looks like the field is not being redrawn. For example if I switch to some other window and back it will work. Neither repaint/eventDrawingBracked nor requestSync doesn't help to redraw the field.

Vadim Gindin wrote:

It looks like the field is not being redrawn. For example if I switch to some other window and back it will work. Neither repaint/eventDrawingBracked nor requestSync doesn't help to redraw the field.

I've fixed it just now. The reason was in focusing problem: FillIn draws selection only if the field has focus, but when popup menu is closed after "Select All" item is clicked the focus wasn't returned to the field. Sorry to trouble.

What is the difference between scroll operations: Scroll Up and Page up?

Vadim Gindin wrote:

What is the difference between scroll operations: Scroll Up and Page up?

Oh, it's just short step ScrollBar.shortStep up and long step up ScrollBar.longStep )

How to calculate correct position of scrollbar thumb using MouseEvent? "Scroll Here" menu item must pose thumb at the point of mouse click. I have a click point and I need to calculate thumb position. There is a ScrollBarGuiImpl.position(..) but it works not as I expected: When I click on scroll pane before thumb and choose "Scroll Here" the thumb always get into last position in the pane (returned by position() method).

Vadim Gindin wrote:

How to calculate correct position of scrollbar thumb using MouseEvent? "Scroll Here" menu item must pose thumb at the point of mouse click. I have a click point and I need to calculate thumb position. There is a ScrollBarGuiImpl.position(..) but it works not as I expected: When I click on scroll pane before thumb and choose "Scroll Here" the thumb always get into last position in the pane (returned by position() method).

Here is an update of my question. "Scroll Here" is the operation where scroll thumb are moved to the point of right mouse button click in a scroll pane. At that thumb is centralized relative to click point (vertically for vertical scroll bar and horizontally for horizontal scroll bar).

So I get click point mousePos and do the following:

   // + or -  depending on mouse click is "after" thumb or "before".
   int mp = owner.getThumbOffset() < 0 ? -1 : 1; 

   if (owner.orientation() == ScrollBar.Orientation.HORIZONTAL)
   {
      mousePos.x = mousePos.x + mp * owner.getThumbDim().width / 2;
   }
   else
   {
      mousePos.y = mousePos.y + mp * owner.getThumbDim().height / 2;
   }

   int pos = owner.position(mousePos);

   owner.notifyScrollListeners(pos);

It takes very approximate, not accurate results. Could you help me to find out correct algorithm? Thanks

Vadim Gindin wrote:

Vadim Gindin wrote:

How to calculate correct position of scrollbar thumb using MouseEvent? "Scroll Here" menu item must pose thumb at the point of mouse click. I have a click point and I need to calculate thumb position. There is a ScrollBarGuiImpl.position(..) but it works not as I expected: When I click on scroll pane before thumb and choose "Scroll Here" the thumb always get into last position in the pane (returned by position() method).

Here is an update of my question. "Scroll Here" is the operation where scroll thumb are moved to the point of right mouse button click in a scroll pane. At that thumb is centralized relative to click point (vertically for vertical scroll bar and horizontally for horizontal scroll bar).

So I get click point mousePos and do the following:
[...]

It takes very approximate, not accurate results. Could you help me to find out correct algorithm? Thanks

Scroll bar position is expressed in "logical" units, when the scroll bar thumb is all the way up/left the position (ScrollBar.position) is 0, when the scroll bar thumb is all the way bottom/right the position is ScrollBar.max.

You have to translate the mouse location to the logical scroll bar position. This should be the expression to get the logical position: travelClick/travelSize * ScrollBar.max.

Where travelSize is a pixel size of the travel interval of the thumb's middle point. scrollBarWidth - 2*scrollBarButtonWidth - thumbWidth for horizontal scroll bar and scrollBarHeight - 2*scrollBarButtonHeight - thumbHeight for vertical scroll bar.

travelClick is a pixel position of the mouse click relative to the start of travel interval, i.e. scrollBarScreenX + scrollBarButtonWidth + 1/2*thumbWidth or scrollBarScreenY + scrollBarButtonHeight + 1/2*thumbHeight for vertical scroll bar.

Hynek Cihlar wrote:

..
You have to translate the mouse location to the logical scroll bar position. This should be the expression to get the logical position: travelClick/travelSize * ScrollBar.max.

Where travelSize is a pixel size of the travel interval of the thumb's middle point. scrollBarWidth - 2*scrollBarButtonWidth - thumbWidth for horizontal scroll bar and scrollBarHeight - 2*scrollBarButtonHeight - thumbHeight for vertical scroll bar.

Can I use ScrollBarGuiImpl methods: getThumbSize() and getButtonSize() for that?

travelClick is a pixel position of the mouse click relative to the start of travel interval, i.e. scrollBarScreenX + scrollBarButtonWidth + 1/2*thumbWidth or scrollBarScreenY + scrollBarButtonHeight + 1/2*thumbHeight for vertical scroll bar.

I.e. travelClick = scrollBarScreenX + scrollBarButtonWidth + 1/2*thumbWidth + mouseClick.x - correct?

Vadim Gindin wrote:

Hynek Cihlar wrote:

..
You have to translate the mouse location to the logical scroll bar position. This should be the expression to get the logical position: travelClick/travelSize * ScrollBar.max.

Where travelSize is a pixel size of the travel interval of the thumb's middle point. scrollBarWidth - 2*scrollBarButtonWidth - thumbWidth for horizontal scroll bar and scrollBarHeight - 2*scrollBarButtonHeight - thumbHeight for vertical scroll bar.

Can I use ScrollBarGuiImpl methods: getThumbSize() and getButtonSize() for that?

Do you mean to create the methods?

travelClick is a pixel position of the mouse click relative to the start of travel interval, i.e. scrollBarScreenX + scrollBarButtonWidth + 1/2*thumbWidth or scrollBarScreenY + scrollBarButtonHeight + 1/2*thumbHeight for vertical scroll bar.

I.e. travelClick = scrollBarScreenX + scrollBarButtonWidth + 1/2*thumbWidth + mouseClick.x - correct?

Assuming mouseClick.x is in the same screen coordinates as scrollBarScreenX the travelClick would be equal to mouseClick.x - (scrollBarScreenX + scrollBarButtonWidth + 1/2*thumbWidth).

Thank you Hynek!

Do you mean to create the methods?

I meant existing methods ScrollBarGuiImpl.getButtonSize() and ScrollBarGuiImpl.getTravelSize().

I've rebased the current branch 2849a. Current revision is 10980.

Vadim Gindin wrote:

Thank you Hynek!

Do you mean to create the methods?

I meant existing methods ScrollBarGuiImpl.getButtonSize() and ScrollBarGuiImpl.getTravelSize().

I can't locate ScrollBarGuiImpl.getButtonSize(), is this a new method in your working copy?

ScrollBarGuiImpl.getTravelSize() returns the scroll bar size less buttons. But I think you need to subtract size of the thumb in addition. Your travel size (or the effective area where the user can click) is delimited by the thumb's middle line as you cannot scroll past the thumb's left or right edge.

Hynek Cihlar wrote:

I can't locate ScrollBarGuiImpl.getButtonSize(), is this a new method in your working copy?

Yes, sorry. I added it.

ScrollBarGuiImpl.getTravelSize() returns the scroll bar size less buttons. But I think you need to subtract size of the thumb in addition. Your travel size (or the effective area where the user can click) is delimited by the thumb's middle line as you cannot scroll past the thumb's left or right edge.

Yes, I do that:

               int travelClick = owner.getButtonSize();
               int travelSize = owner.getTravelSize();

               NativePoint loc = owner.physicalLocation();

               if (owner.orientation() == ScrollBar.Orientation.HORIZONTAL)
               {
                  travelSize -= owner.getThumbDim().width;
                  travelClick += loc.x + owner.getThumbDim().width / 2;
                  travelClick = mousePos.x - travelClick;
               }
               else
               {
                  travelSize -= owner.getThumbDim().height;
                  travelClick += loc.y + owner.getThumbDim().height / 2;
                  travelClick = mousePos.y - travelClick;
               }

               int resThumbLoc = owner.getMax() * travelClick / travelSize;

               owner.notifyScrollListeners(resThumbLoc);

Is that what you've meant?

Vadim Gindin wrote:

Hynek Cihlar wrote:

I can't locate ScrollBarGuiImpl.getButtonSize(), is this a new method in your working copy?

Yes, sorry. I added it.

ScrollBarGuiImpl.getTravelSize() returns the scroll bar size less buttons. But I think you need to subtract size of the thumb in addition. Your travel size (or the effective area where the user can click) is delimited by the thumb's middle line as you cannot scroll past the thumb's left or right edge.

Yes, I do that:
[...]

Is that what you've meant?

You should replace owner.physicalLocation() with owner.screenPhysicalLocation().

Hynek, sometimes scrollbar.orientation() returns incorrect value: for example HORIZONTAL when I'm clicking on a vertical scroll bar.

Your approach doesn't work if number of text lines (if we talking about vertical scroll bar) is greater than editor lines less than 100%. For example editor lines is 5 and text lines is 8... In such cases scrollbar can have incorrect values. for example MAX can be 0, ORIENTATION could be incorrect. Did you faced with such errors?

By the way, If I'm just typing '111111111111111111....' at some moment I got an exception:

Caused by: java.lang.StringIndexOutOfBoundsException: String index out of range: 52
        at java.lang.String.substring(String.java:1951)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.computeHorizontalDelta(EditorGuiImpl.java:2425)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.access$4(EditorGuiImpl.java:2421)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl$EditorContent.draw(EditorGuiImpl.java:2655)
        at com.goldencode.p2j.ui.client.widget.AbstractContainer.draw(AbstractContainer.java:412)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.access$4(BorderedPanelGuiImpl.java:1)
        ..

Also when the number of characters in a single lines is equal to editor character width and the user has typed another one characters: PROGRESS sometimes makes a "shift": extending "line buffer" to several characters at a time. If the user continue typing in this line it happens as usual, but P2J makes that "shift" each next typed character. If the user continues typing, it will get the same exception:

Caused by: java.lang.StringIndexOutOfBoundsException: String index out of range: 52
        at java.lang.String.substring(String.java:1951)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.computeHorizontalDelta(EditorGuiImpl.java:2425)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.access$4(EditorGuiImpl.java:2421)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl$EditorContent.draw(EditorGuiImpl.java:2655)

Vadim Gindin wrote:

Hynek, sometimes scrollbar.orientation() returns incorrect value: for example HORIZONTAL when I'm clicking on a vertical scroll bar.

Please post a code sample where you can see this and steps how to reproduce it.

Your approach doesn't work if number of text lines (if we talking about vertical scroll bar) is greater than editor lines less than 100%. For example editor lines is 5 and text lines is 8... In such cases scrollbar can have incorrect values. for example MAX can be 0, ORIENTATION could be incorrect. Did you faced with such errors?

Please post a code sample where you can see this and steps how to reproduce it.

By the way, If I'm just typing '111111111111111111....' at some moment I got an exception:
[...]
Is it known bug?

Also when the number of characters in a single lines is equal to editor character width and the user has typed another one characters: PROGRESS sometimes makes a "shift": extending "line buffer" to several characters at a time. If the user continue typing in this line it happens as usual, but P2J makes that "shift" each next typed character. If the user continues typing, it will get the same exception:
[...]

I suggest you try to search in Redmine whether you find opened issues for the above, if not create new issues and put Greg as a watcher there.

Here is the test:

def var editor1 as character view-as editor size 20 by 5 label "Editor 1".
def button btn-exit label "Exit".

def frame frame1 editor1 skip(1) btn-exit with side-labels centered.

editor1:SCROLLBAR-HORIZONTAL = true.
editor1:SCROLLBAR-VERTICAL = true.

enable all with frame frame1.

on entry of editor1 
do:
   message "Focus editor1". 
end.

on entry of frame frame1  
do:
   message "Focus frame1". 
end.

wait-for choose of btn-exit.

Incorrect values happen from sometimes, not every time. Scenario is simple. Enter lines:

1
2
3
..
9

Hynek Cihlar wrote:

Vadim, see the attached diff. It fixes two issues, (1) an abend when doubleclicking inside an empty editor and (2) scroll bar attempts to scroll outside of the scroll interval causing an abend. Please integrate the diff into 2849a.

Done. rev 10982.

In WindowGuiImpl.initScrollPopup() you save the value of owner parameter in the cached popup (scrollPopup). You should either recreate the scrollPopup in every execution of WindowGuiImpl.initScrollPopup() or update the scroll bar reference in the cached popup.

I already save owner id in a popup menu config in every initScrollPopup() call. Do you mean something else?

Vadim Gindin wrote:

In WindowGuiImpl.initScrollPopup() you save the value of owner parameter in the cached popup (scrollPopup). You should either recreate the scrollPopup in every execution of WindowGuiImpl.initScrollPopup() or update the scroll bar reference in the cached popup.

I already save owner id in a popup menu config in every initScrollPopup() call. Do you mean something else?

initScrollPopup() is called for the first time for vertical scroll-bar, MenuGuiImpl is created and initialized with the vertical scroll-bar reference, reference to MenuGuiImpl is cached in WindowGuiImpl.scrollPopup. initScrollPopup() is called for the second time with horizontal scroll-bar, but the method returns the cached MenuGuiImpl previously initialized with the vertical scroll-bar.

1. System popups are implemented.
2. The system popup of window title bar displays icons for some of items. Icons are not supported/implemented at this moment.
3. The items of system popup of window title bar is not implemented, (i.e. actions).
4. ENTRY event, focusing widget when it is clicked by RMB. Once again, It looks like widgets do not receive ENTRY event, are not highlighted when clicked by RMB. I'd proposed short solution (note 9). See the first notes in this task for details.

What should I do next? What to do with RMB ignoring?

Vadim Gindin wrote:

Here is the current situation.

1. System popups are implemented.
2. The system popup of window title bar displays icons for some of items. Icons are not supported/implemented at this moment.
3. The items of system popup of window title bar is not implemented, (i.e. actions).
4. ENTRY event, focusing widget when it is clicked by RMB. Once again, It looks like widgets do not receive ENTRY event, are not highlighted when clicked by RMB. I'd proposed short solution (note 9). See the first notes in this task for details.

What should I do next? What to do with RMB ignoring?

Vadim, were you able to resolve all the scrolling issues for the "Scroll Here" functionality? Even the "sometimes scrollbar.orientation() returns incorrect value: for example HORIZONTAL when I'm clicking on a vertical scroll bar"?

Hynek Cihlar wrote:

Vadim Gindin wrote:

Here is the current situation.

1. System popups are implemented.
2. The system popup of window title bar displays icons for some of items. Icons are not supported/implemented at this moment.
3. The items of system popup of window title bar is not implemented, (i.e. actions).
4. ENTRY event, focusing widget when it is clicked by RMB. Once again, It looks like widgets do not receive ENTRY event, are not highlighted when clicked by RMB. I'd proposed short solution (note 9). See the first notes in this task for details.

What should I do next? What to do with RMB ignoring?

Vadim, were you able to resolve all the scrolling issues for the "Scroll Here" functionality? Even the "sometimes scrollbar.orientation() returns incorrect value: for example HORIZONTAL when I'm clicking on a vertical scroll bar"?

Also, what about the "editor lines is 5 and text lines is 8... In such cases scrollbar can have incorrect values. for example MAX can be 0, ORIENTATION could be incorrect."? Were you able to resolve this one?

Hynek Cihlar wrote:

..
Vadim, were you able to resolve all the scrolling issues for the "Scroll Here" functionality? Even the "sometimes scrollbar.orientation() returns incorrect value: for example HORIZONTAL when I'm clicking on a vertical scroll bar"?

Yes, the reason was in internal caching of owner variable. Thank you!

..
Also, what about the "editor lines is 5 and text lines is 8... In such cases scrollbar can have incorrect values. for example MAX can be 0, ORIENTATION could be incorrect."? Were you able to resolve this one?

After I've fixed previous error I didn't met that error anymore. Probably it was also related with internal caching of owner.

Vadim, are all the points from note 19 resolved? If not, please proceed with those. Should you have any questions please go ahead and ask.

Hynek Cihlar wrote:

Greg Shah wrote:

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim, I think that a sensitive widget should still receive focus when it is right clicked (also middle clicked?) (even if its context menu is shown). But the code change in ThinClient will only allow left-click, right?

Hynek, you wrote this earlier. What did you mean? I could only found that from all widgets only FRAME and WINDOW can receive ENTRY event. No widget is highlighted as a result of RMB.

Vadim: Please create a new sub-task of #2677 for the java.lang.StringIndexOutOfBoundsException: String index out of range: 52 that you described in note 47. Make sure that the 2 recreates can be done using code already checked in to testcases/uast/. If you have local modifications, post the actual code into the new task. Make me and Constantin watchers.

Vadim Gindin wrote:

Hynek Cihlar wrote:

Greg Shah wrote:

I'm OK with the changes.

Hynek/Constantin: any concerns with the ThinClient change?

Vadim, I think that a sensitive widget should still receive focus when it is right clicked (also middle clicked?) (even if its context menu is shown). But the code change in ThinClient will only allow left-click, right?

Hynek, you wrote this earlier. What did you mean? I could only found that from all widgets only FRAME and WINDOW can receive ENTRY event. No widget is highlighted as a result of RMB.

What do you mean by "widget is highlighted"?

When you right-click in Fillin or Editor widget the text cursor is transferred in the widget, also when you select "Paste" from the context menu any text in the clipboard is pasted in the widget. This makes me to believe that the widget is focused. Please note that I am not mentioning the ENTRY event, that is indeed send only after the context menu is dismissed.

Also consider the case when two fillins (or editors) are right-clicked in sequence, that is while a context menu for fillin A is displayed you right-click in fillin B. ENTRY event is sent to fillin A when right-clicked in fillin B. This could seem a bit non-intuitive.

Vadim Gindin wrote:

I've checked again FillIn and Editor widgets. It behave in the following way (when right mouse button is clicked):

1. The cursor is set to the field (not blinked).
2. System popup menu is displayed (standard default menu with common operations like COPY/CUT/PASTE).
3. When user clicks somewhere outside and popup menu is closed - entry event is sent to a widget and it's cursor starts blinking.

Issues 1 and 2 happen during processing of right mouse button click event. Issue 3 happen during processing the next event, that leads to popup menu close.

So it means, that at the moment of processing of right mouse button click: there is no difference for widgets: all of them do not send ENTRY event. It happen only when the next click (next event) is processed (and popup menu is closed).

I still don't think that we fully understand what is going on here.

1. There are multiple ways to dismiss the popup:

• select an item in the popup
• press ESC key
• click LMB or RMB somewhere outside of the popup
  • still inside of the 4GL window, but inside the editor or fillin that was the popup menu widget
  • still inside of the 4GL window, but inside an editor or fillin that was NOT the popup menu widget
  • still inside of the 4GL window, but inside some other widget or frame
  • still inside of the 4GL window, but outside of any widget
  • outside the 4GL window that had the popup, but inside a different 4GL window
  • outside of any 4GL window

Please determine the behavior in each case and post it here. Pictures may help. I suspect that some of the cases will not generate an ENTRY event because we don't leave the widget.

2. It is not clear where the focus is when you RMB to bring up the popup. Have you tested when the focus is on a different widget from the one that you click on with RMB? Such a case might cause focus change and ENTRY event before the popup is shown. It should be tested both ways (focus already on the popup widget at time of RMB and focus not on the popup widget at time of RMB).

3. It is not clear which widget has the focus and which widget gets the ENTRY event, from your comment.

There are possible 3 variants:

1. Leave the current change, that add right mouse button ignoring for all widgets and fix FillIn and Editor cases later.
2. Add widget instance check for some of widgets: toggle-box, combo-box, radio-set, button or just for my toggle-box instead of current change.
3. Add some method like isRMBFocusSupported() to Widget interface.

What is more preferable in current situation?

I still don't think we know that this is needed. But for future reference, IF we do need to differentiate some RMB behavior, we would certainly pick option 3. We do not what to encode instance checks if we have a clean way to let a widget tell us the answer. We also don't want to have to fix fillin or editor later.

Behaviour in order of appearance. There are 2 different cases: default and overridden popup menu.

FillIn, Editor are assumed as "widget" further.

Default popup menu:
   widget gets the cursor
   popup is opened
   action that leads to popup close: ESC, clicking somewhere in different places, choosing menu item and so on.
   popup is closed
   widget gets the ENTRY event only if:
      it has not the cursor before 
      closing action wasn't RMB click in the same widget, that leads to another popup opening 

Overridden menu:
   widget DOES NOT get the cursor except the action that led to popup close was LMB click inside this widget.
   popup is opened
   action that leads to popup close: ESC, clicking somewhere in different places, choosing menu item and so on.
   popup is closed
   widget DOES NOT get the ENTRY event except the action that led to popup close was LMB click inside this widget.

The cursor is displayed only if the window containing focused widget is active. In other words, if we're switching to some other window, the cursor disappears in old window and appears back when window become active again.

P.S. It's hard to post screenshots: too many cases. GIF's would be better..

Default popup menu:
widget gets the cursor

Having the cursor is typically the same thing as having the focus.

If the widget doesn't already have the cursor, it will receive the focus without an ENTRY event?

action that leads to popup close: ESC, clicking somewhere in different places, choosing menu item and so on.
popup is closed
widget gets the ENTRY event only if:
it has not the cursor before
closing action wasn't RMB click in the same widget, that leads to another popup opening

You are saying that clicking the mouse elsewhere still results in an ENTRY event for the widget? In other words, the mouse event is "eaten" and does not switch away from the widget. It just dismisses the popup.

Greg Shah wrote:

Default popup menu:
widget gets the cursor

Having the cursor is typically the same thing as having the focus.

If the widget doesn't already have the cursor, it will receive the focus without an ENTRY event?

Sorry. It's my mistake. The widget get the cursor first time => it will get ENTRY event after popup will be closed in all cases. But if the widget already has the cursor - it will not get ENTRY event.

action that leads to popup close: ESC, clicking somewhere in different places, choosing menu item and so on.
popup is closed
widget gets the ENTRY event only if:
it has not the cursor before
closing action wasn't RMB click in the same widget, that leads to another popup opening

You are saying that clicking the mouse elsewhere still results in an ENTRY event for the widget?

Yes.

In other words, the mouse event is "eaten" and does not switch away from the widget. It just dismisses the popup.

What do you mean?

In other words, the mouse event is "eaten" and does not switch away from the widget. It just dismisses the popup.

What do you mean?

By "eaten" I mean that the system ignores the normal behavior of a click and uses the click only for dismissing the popup. The system "eats" the click and acts like it did not have any effect on the normal target widget or window over which the click occurred.

The problem here is that we must detect a click outside of the popup but the click is really needing to be processed by the popup for dismiss purposes.

Greg Shah wrote:

The problem here is that we must detect a click outside of the popup but the click is really needing to be processed by the popup for dismiss purposes.

I think this is the same case as with combo drop-down. We do not handle mouse-click there but rather rely on WindowActivated event sent by GUI driver notifying about the overlay window being deactivated.

I think this is the same case as with combo drop-down. We do not handle mouse-click there but rather rely on WindowActivated event sent by GUI driver notifying about the overlay window being deactivated.

Agreed.

Vadim: do you now have enough specifics about what the 4GL does to propose a solution?

I recall that at this step menus are implemented in the same window. In other words opened popup window is still in the widget hierarchy of active window. Therefore not every click will raise WindowActivated event: only the click in the other window. May be it is needed to implement menus in a separate window first to go further in the proposed way. What do you think?

P.S. Greg, sorry for stupid question. Could you clarify (concrete example): what normal is ignored in what case?

So, can I start to implement menus in a separate window?

I suppose it would be better to commit system menus and finish ENTRY event in some other task?

May be it is needed to implement menus in a separate window first to go further in the proposed way. What do you think?

Yes, this will be needed for the proper "dismiss" logic. We already have task #2970 for implementing menus using the overlay window. That work will be a bit tricky and it may be best to have Eugenie or Hynek take that task.

What can you complete in this task, without that support?

So, can I start to implement menus in a separate window?

No, I want you to fix other issues first and leave the separate window support to someone else.

Could you clarify (concrete example): what normal is ignored in what case?

If you click on some other widget in the same window (for example, a button), then the button would NOT receive the click because the click is "eaten" by the dismissal of the popup.

I suppose it would be better to commit system menus and finish ENTRY event in some other task?

Perhaps you can implement the listener for window activation (and the proper behavior for ENTRY)? I understand that until #2970 is done, some cases won't work, but it would be better if we don't have to revisit this.

Greg Shah wrote:

..
If you click on some other widget in the same window (for example, a button), then the button would NOT receive the click because the click is "eaten" by the dismissal of the popup.

I've checked it again. There is no "dismiss" logic. Whenever I click on some other widget in the same window or in a widget in other window, that widget RECEIVES the click. It happen right after popup is closed and ENTRY event for popup widget.

Assume we have the following example:

Window1
  FillIn (with default popup and ENTRY trigger)

Window2
  Button (with ENTRY trigger)

Assume all triggers just call MESSAGE statement with some string.

Scenario. I click RMB on FillIn and popup is opened (Window1). After that I click LMB on Button in Window2. Than here will be the following behaviour:

   ...
   popup is closed
   ENTRY of FillIn (in Window1)
   ENTRY of Button (in Window2)

By the way there is the following peculiarity: all messages will be printed in a message area of Window1.

1. inheritance: MessageAreaGuiImpl inherits ScrollPaneGuiImpl
2. parent-child relationship: EditorGuiImpl is a parent of it's scrollbars (ScrollBarGuiImpl)
3. composition: Frame contains the field frameScroll.

I've committed the following changes: isRMBEntrySupported method(), added corresponding check to ThinClient.processProgressEvent() for maySwitchFocus, added processing of Window deactivation to send ENTRY event to a widget owning previously opened system popup and some other small scroll bar fixes. rev #10991

I've added TODO for the case when MENU will be implemented in a separate WINDOW, so that code should work also for the case when user clicks the other widget in the same window as a widget owning system popup.

It looks like there are no other changes needed for this task. So please review the code.

Code Review Task Branch 2849a Revision 10991

1. Please rebase this to the latest trunk revision 10971.

2. The use of AWT-specific code (MouseEvent) in ThinClient, ScrollBar is not appropriate. In the ThinClient case, wouldn't it be better to hide the AWT references in the MouseEvt class instead of adding new AWT dependencies to code (like ThinClient) which is supposed to be common code? Shouldn't the MouseEvt be able to tell us that the event was caused by an LMB click? There should be a better solution to the ScrollBar case too. Do NOT add references to AWT from common code.

3. It is not clear to me if the changes to how invalidateSelection() is called in the EditorGuiImpl.mouseClicked() and FillInGuiImpl.mouseClicked() methods are correct. Please document (here) some reasoning for the change.

4. I think the code in WindowGuiImpl.initEditorPopup() and WindowGuiImpl.initScrollPopup() and probably even WindowGuiImpl.createPopup() should be moved to separate classes. WindowGuiImpl should not have all the low-level menu creation knowledge in it. It should be able to create instances of those other classes and use them as needed.

5. The package private static method MenuGuiImpl.createDynamicPopupMenu() should be located after the protected static methods.

6. In ScrollBarGuiImpl, the getTravelSize() getButtonSize() and getThumbOffset() package private methods should be placed after the protected methods.

7. There is an explicit import of AWT in MenuItemGuiImpl that was previously import java.awt.event.MouseEvent;. Although we usually want these to be wildcards, in this case it is preferred to leave this explicit since it is an unwanted dependency. We don't want people thinking that using other AWT classes from here is acceptable. Please place a comment there to explain this and remove the wildcard. Please also fix the same problem in WindowGuiImpl.

8. Your IDE has converted some wildcard imports into explicit imports. Please fix this (see ThinClient, WindowGuiImpl).

9. ToggleBoxGuiImpl.draw() has some debugging/logging code that should be removed.

10. BrowseGuiImpl and ToggleBoxGuiImpl are missing a history entry.

11. ScrollBarGuiButton and ScrollBarGuiImpl should not have 2 history entries for the same task branch.

12. CaptionButtonType needs its copyright date extended to 2015.

I've rebased branch 2849a with the latest trunk revision. Current branch revision is 10992.

Greg Shah wrote:

..
3. It is not clear to me if the changes to how invalidateSelection() is called in the EditorGuiImpl.mouseClicked() and FillInGuiImpl.mouseClicked() methods are correct. Please document (here) some reasoning for the change.
..

I'd added the following changed there: reset selection (call invalidateSelection()) only if clicked mouse button is not menu mouse button (i.e. not RMB). It is needed for clipboard operations, such as COPY, that operates with selection. Simple scenario: user selected some text in a field, clicked RMB and choose COPY => selection is not invalidated and selected text is copied to clipboard.

Code Review Task Branch 2849a Revision 10995

1. In EditorPopupGuiImpl, this code:

public class EditorPopupGuiImpl extends MenuGuiImpl
{
   /** CUT/COPY/PASTE/SELECT_ALL items from editor system popup */
   private MenuItemGuiImpl cutOpt, copyOpt, pasteOpt, selectAllOpt;

should be this:

public class EditorPopupGuiImpl
extends MenuGuiImpl
{
   /** CUT item from editor system popup. */
   private MenuItemGuiImpl cutOpt;

   /** COPY item from editor system popup. */
   private MenuItemGuiImpl copyOpt;

   /** PASTE items from editor system popup. */
   private MenuItemGuiImpl pasteOpt;

   /** SELECT_ALL item from editor system popup. */
   private MenuItemGuiImpl selectAllOpt;

2. For both ScrollPopupGuiImpl and WindowTitlePopupGuiImpl, please break the extends clause onto the next line as per the coding standards.

Fixed. Can I ran regression testing?

Yes, you can start testing.

Hynek and Constantin: please review.

EditorGuiImpl.mouseClicked()
The wnd.initEditorPopup() call is fishy and I think may not work in a DIALOG-BOX. Unless you already haven't please also test the popup menus in a DIALOG-BOX (especially the case when no WindowGuiImpl is visible).
Same for FillInGuiImpl or ScrollBarGuiImpl.onButtonClick().

In MenuGuiImpl, why to use reflection instead of just calling the constructors of the specific MenuGuiImpl descendants?

MenuItemGuiImpl.createDynamicItem()
You should pass default config instance to afterConfigUpdate() to satisfy its contract. So something like this item.afterConfigUpdate(new MenuItemConfig()).

In WindowGuiImpl.processEvent() the expression in if statement is always true: if (!(owningWidget instanceof Window)).

Is the WindowTitlePopupGuiImpl created in WindowGuiImpl.init() deregistered from the driver (with the call to GuiDriver.deregisterWidget())?

Hynek Cihlar wrote:

EditorGuiImpl.mouseClicked()
The wnd.initEditorPopup() call is fishy and I think may not work in a DIALOG-BOX. Unless you already haven't please also test the popup menus in a DIALOG-BOX (especially the case when no WindowGuiImpl is visible).
Same for FillInGuiImpl or ScrollBarGuiImpl.onButtonClick().

Well, yes these popup's are designed to be supported exactly by WindowGuiImpl.. I'm testing

In MenuGuiImpl, why to use reflection instead of just calling the constructors of the specific MenuGuiImpl descendants?

There is a static method createSystemPopupMenu().. I can get rid of it, if you insist. I can create 3 methods, for example, instead of the one or just transfer newly created instance right to the method instead of class object.

MenuItemGuiImpl.createDynamicItem()
You should pass default config instance to afterConfigUpdate() to satisfy its contract. So something like this item.afterConfigUpdate(new MenuItemConfig()).

Such call is already there.

In WindowGuiImpl.processEvent() the expression in if statement is always true: if (!(owningWidget instanceof Window)).

In most cases owningWidget will be concrete widget like EDITOR or FILLIN, but if scroll bar is owned by MESSAGE_AREA this method will return WINDOW instance, because MessageAreaGuiImpl is a descendant of ScrollPaneGuiImpl.

Is the WindowTitlePopupGuiImpl created in WindowGuiImpl.init() deregistered from the driver (with the call to GuiDriver.deregisterWidget())?

It looks. it is really missed. I'll add it. How to do it better? There is no field for it in WindowGuiImpl class. I could just find WindowTitlePopupGuiImpl instance in contentPane's instances list. What do you think?

Vadim Gindin wrote:

Hynek Cihlar wrote:

In MenuGuiImpl, why to use reflection instead of just calling the constructors of the specific MenuGuiImpl descendants?

There is a static method createSystemPopupMenu().. I can get rid of it, if you insist. I can create 3 methods, for example, instead of the one or just transfer newly created instance right to the method instead of class object.

I don't insist, just a suggestion :-). The problem with reflection is that you lose type safety and the support of IDE/tools.

A real-life example, when I wanted to find all references of the MenuGuiImpl ctors using Eclipse's Search -> References (to see where it was instantiated) I didn't find any. I had to search for it manually which requires a magnitude more effort.

MenuItemGuiImpl.createDynamicItem()
You should pass default config instance to afterConfigUpdate() to satisfy its contract. So something like this item.afterConfigUpdate(new MenuItemConfig()).

Such call is already there.

afterConfigUpdate() is called with the active config, instead you should pass a config that is free of any changes for the current scope. So that code like the following works:

if (this.config.visible != beforeUpdate.visible)
{
   setVisible(...);
}

In WindowGuiImpl.processEvent() the expression in if statement is always true: if (!(owningWidget instanceof Window)).

In most cases owningWidget will be concrete widget like EDITOR or FILLIN, but if scroll bar is owned by MESSAGE_AREA this method will return WINDOW instance, because MessageAreaGuiImpl is a descendant of ScrollPaneGuiImpl.

I see.

Is the WindowTitlePopupGuiImpl created in WindowGuiImpl.init() deregistered from the driver (with the call to GuiDriver.deregisterWidget())?

It looks. it is really missed. I'll add it. How to do it better? There is no field for it in WindowGuiImpl class. I could just find WindowTitlePopupGuiImpl instance in contentPane's instances list. What do you think?

I would say keep it simple, follow the pattern of WindowGuiImpl.editorPopup and WindowGuiImpl.scrollPopup, store the reference in a class field and deregister manually in WindowGuiImpl.destroy(). This code will probably change anyway when popup menu moves to a dedicated window.

I've tested DIALOG-BOX. System popups are really don't work for it. So I'm adding that support to DialogBox in the same manner as for WindowGuiImpl, but is there real significance to have one instance of system popup of particular type for one window/dialog-box. May be it is more correct to have one instance of system popup of particular type for all application. What do you think?

Vadim Gindin wrote:

I've tested DIALOG-BOX. System popups are really don't work for it. So I'm adding that support to DialogBox in the same manner as for WindowGuiImpl, but is there real significance to have one instance of system popup of particular type for one window/dialog-box. May be it is more correct to have one instance of system popup of particular type for all application. What do you think?

I think it would be best to move the popup implementation from WindowGuiImpl out to a separate class, PopupMenuManager for example, and forward window events from TopLevelWindow to it. This would also prepare the ground for moving popups to dedicated top-level windows.

Does anybody know why in common class TopLevelWindow the following GUI-specific statement is used: GuiDriver gd = (GuiDriver) OutputManager.getDriver(); (see TopLevelWindow.processEvent() method) ?

If it is OK I would also use GuiDriver in this class for example for destroying system popups.

Vadim Gindin wrote:

Does anybody know why in common class TopLevelWindow the following GUI-specific statement is used: GuiDriver gd = (GuiDriver) OutputManager.getDriver(); (see TopLevelWindow.processEvent() method) ?

If it is OK I would also use GuiDriver in this class for example for destroying system popups.

Yes we attempt not to put driver-specific code into common code, but there are exceptions. In the case you pointed out the code is executed only in GUI mode.

Hynek Cihlar wrote:

..
Yes we attempt not to put driver-specific code into common code, but there are exceptions. In the case you pointed out the code is executed only in GUI mode.

1. I need to call gd.deregisterWidget(popup) in window.destroy() method. I can do it in TopLevelWindow or GUI descendants: WindowGuiImpl, DialogBoxWindow. What is preferrable?

2. I have the following procedure for dialog-box without WindowGuiImpl:

def var editor1 as character view-as editor size 20 by 10 label "Editor 1".

def button btn-exit label "Exit".

def frame frame1  editor1 skip(1) btn-exit  with  side-labels centered view-as dialog-box.

frame frame1:height-pixels = 300.
frame frame1:width-pixels = 400.
frame frame1:title = "Dialog".

editor1:SCROLLBAR-VERTICAL = true.
editor1:SCROLLBAR-HORIZONTAL = true.

enable all with frame frame1.

wait-for choose of btn-exit.

There is no visible WINDOW exception DIALOG-BOX itself, but DialogBoxWindow.window() get an owner property that is some mysterious WindowGuiImpl object (realized=true, hidden=false, visible=false, id=1).

Is it OK?

I need to define mouse event source (calling findMouseSource method from a owning WINDOW). I'll probably need to call that method exactly for DialogBox.. How to properly get the window from Menu.mouseClicked() trigger? I'd tried window(), but it returns that mysterious WindowGuiImpl instead of DialogBox.

Vadim Gindin wrote:

Hynek Cihlar wrote:

..
Yes we attempt not to put driver-specific code into common code, but there are exceptions. In the case you pointed out the code is executed only in GUI mode.

1. I need to call gd.deregisterWidget(popup) in window.destroy() method. I can do it in TopLevelWindow or GUI descendants: WindowGuiImpl, DialogBoxWindow. What is preferrable?

If you have the popup field declared in TopLevelWindow then you should deregister it in TopLevelWindow.

2. I have the following procedure for dialog-box without WindowGuiImpl:
[...]

There is no visible WINDOW exception DIALOG-BOX itself, but DialogBoxWindow.window() get an owner property that is some mysterious WindowGuiImpl object (realized=true, hidden=false, visible=false, id=1).

Is it OK?

Yes, it is OK. The mysterious window is a default window (WidgetId.DEFAULT_WINDOW_ID). It is realized on client startup and has a special meaning to 4GL.

I need to define mouse event source (calling findMouseSource method from a owning WINDOW). I'll probably need to call that method exactly for DialogBox.. How to properly get the window from Menu.mouseClicked() trigger? I'd tried window(), but it returns that mysterious WindowGuiImpl instead of DialogBox.

The event object for mouse click holds a source widget (MouseEvt.source()). Call topLevelWindow() on it to get to the nearest top-level window of the event source widget.

I'm trying to fix the following bug:
When I open popup menu in EDITOR in a standard WINDOW it is shown at the point of mouse click, but when I open menu in DIALOG-BOX in the test procedure (see the note #89) - popup menu is shown in the point click+[4,23].

It leads to incorrect mouse source defining. It seems that title height (23) and border (4) was substracted extra time or base point in both cases are different. Any thoughts why it could be?

Vadim Gindin wrote:

I'm trying to fix the following bug:
When I open popup menu in EDITOR in a standard WINDOW it is shown at the point of mouse click, but when I open menu in DIALOG-BOX in the test procedure (see the note #89) - popup menu is shown in the point click+[4,23].

It leads to incorrect mouse source defining. It seems that title height (23) and border (4) was substracted extra time or base point in both cases are different. Any thoughts why it could be?

Eugenie, does it ring a bell? If I remember correctly you were solving some mouse-click offset issues when implementing overlay windows for combo-box drop-downs.

Eugenie, does it ring a bell?

?

If I remember correctly you were solving some mouse-click offset issues when implementing overlay windows for combo-box drop-downs.

No, the problem was in finding correct absolute location of the overlay window to show. The mouse coord itself was not involved. And displayPhysicalLocation() method is used.

Vadim Gindin wrote:

I'm trying to fix the following bug:
When I open popup menu in EDITOR in a standard WINDOW it is shown at the point of mouse click, but when I open menu in DIALOG-BOX in the test procedure (see the note #89) - popup menu is shown in the point click+[4,23].

It leads to incorrect mouse source defining. It seems that title height (23) and border (4) was substracted extra time or base point in both cases are different. Any thoughts why it could be?

I did a quick test, created a simple frame in a regular window and in a DIALOG-BOX. Clicking on the same relative positions gives me the same mouse event positions traced in TopLevelWindow.processEvent().

Do you see the same problem also in trunk? If so please post a code sample.

Hynek Cihlar wrote:

..
I did a quick test, created a simple frame in a regular window and in a DIALOG-BOX. Clicking on the same relative positions gives me the same mouse event positions traced in TopLevelWindow.processEvent().

Did you do that in trunk?

Do you see the same problem also in trunk? If so please post a code sample.

System popups are not committed to trunk yet, so I can't check it, except such tracing as you already did.

Vadim Gindin wrote:

Hynek Cihlar wrote:

..
I did a quick test, created a simple frame in a regular window and in a DIALOG-BOX. Clicking on the same relative positions gives me the same mouse event positions traced in TopLevelWindow.processEvent().

Did you do that in trunk?

I tested it in 2849a.

Do you see the same problem also in trunk? If so please post a code sample.

System popups are not committed to trunk yet, so I can't check it, except such tracing as you already did.

If you see it during tracing, what is the test procedure, where do you point your mouse, where do you trace the mouse events?

Hynek, sorry. I forgot to commit my current changes to 2849a. That also means that the reason could be in some of my changes. Could you check it again with these changes. Testing procedure is defined in the note 89. Branch revision 10999.

Vadim I believe the problem with the popup offset is in TopLevelWindow.initEditorPopup(). The mouse event point is in screen coordinates (origin is the top-left corner of the top-level window) and it is directly assigned to editorPopup. But because editorPopup is attached to the widget tree of its owner its origin is relative to the container it is attached to. So the event coordinates must be adjusted to account for this difference.

You can calculate the offset by subtracting the physical location from the screen physical location of the container (Widget.parent()) the popup is attached to.

What window the method GuiDriver.selectWindow(int windowId) can accept? Can it be DialogBox or only Window id?

If it must be only Window, should I use dialogBox.owner.id for selectWindow call?

Can dialogBox.owner be null?

Vadim Gindin wrote:

What window the method GuiDriver.selectWindow(int windowId) can accept? Can it be DialogBox or only Window id?

If it must be only Window, should I use dialogBox.owner.id for selectWindow call?

You can pass only windows that are known (registered) with the driver, see GuiDriver.registerWindow() and GuiDriver.registerChildWindow(). Currently these are WindowGuiImpl, ModalWindow and OverlayWindow.

So yes, you should pass the id of DialogBox to GuiDriver.selectWindow(int windowId).

Can dialogBox.owner be null?

It shouldn't. Even in the case when the dialog box is the only visible top-level window, its owner should be the default window.

I faced with the following problem. Window title popup menu is always shown under title bar for DialogBoxWindow. I've found that titleBar for ModalWindow is not added to contentPane unlike it happen for WindowGuiImpl. In other words title bar popup and title bar itself are contained in a contentPane.widgets for WindowGuiImpl and I can manage those order by changing those place in contentPane.widgets set using moveToTop(), moveToBottom() methods for example. But it doesn't work for DialogBoxWindow where popup is contained in contentPane.widgets but title bar is not contained there.

Could you give me some advice?

Vadim Gindin wrote:

I faced with the following problem. Window title popup menu is always shown under title bar for DialogBoxWindow. I've found that titleBar for ModalWindow is not added to contentPane unlike it happen for WindowGuiImpl. In other words title bar popup and title bar itself are contained in a contentPane.widgets for WindowGuiImpl and I can manage those order by changing those place in contentPane.widgets set using moveToTop(), moveToBottom() methods for example. But it doesn't work for DialogBoxWindow where popup is contained in contentPane.widgets but title bar is not contained there.

Could you give me some advice?

If you need to place the popup always in the same container with the title bar, you can use GuiWindow.getTitleBar() to get the title bar and call parent() on it to get the desired container.

I've fixed popup imlementation for dialog boxes. Please review. Revision 11005.

Some items in window title bar popup have icons, such as Close, Maximize, Minimize, Restore. Should I implement icon support for menu items now?

Vadim Gindin wrote:

I've fixed popup imlementation for dialog boxes. Please review. Revision 11005.

• MousePopupable.drawPopup()
  I think this code
  

        if (window instanceof DialogBoxWindow)
           window = ((DialogBoxWindow) window).getOwner();
  

  
  is not correct. You may end up with a wrong window. Instead just use the window returned by widget.topLevelWindow() or better yet use popup.topLevelWindow() as you do later in the method.
  Also instead of winCfg.id do window.getId().

• It would be better to implement the popups outside of the window class hierarchy. The approach I would take is to create a new class managing all popups and use the abstract interfaces (if possible) of other widgets to setup the popup.
  So instead of this
  window.createWindowTitlePopup(...)
  you would use
  popupManager.createWindowTitlePopup(GuiWindow window)
  But I wouldn't bother with this right now as the popups will have to be reworked to be displayed in separate top-level windows.

Some items in window title bar popup have icons, such as Close, Maximize, Minimize, Restore. Should I implement icon support for menu items now?

I've fixed the first remark... What about others?

Some items in window title bar popup have icons, such as Close, Maximize, Minimize, Restore. Should I implement icon support for menu items now?

No, we will wait on this. Please create a new task.

I need you to finish the current menu tasks which are needed to make menus stable and function properly. That work has been going on for months and we must finish it very quickly.

I've fixed the first remark... What about others?

As Hynek notes, we have some significant changes coming for the separate window implementation. Don't change the popup implementation at this time.

Are there any problems which are unfinished in this task?

Vadim, please rebase the branch to latest trunk.

1. have you tested this with the Web client?
2. some classes are missing history entries
3. ScrollBar.onButtonClick - is missing javadoc for the e param
4. ScrollBar.notifyScrollListeners - move it into the correct location
5. MouseHandler has no functional changes
6. DialogBoxwindow - the java.awt.event import is not needed
7. EditorGuiImpl.mouseClicked - you moved the invalidateSelection(); into the if (@hasFocus()) block - please explain why you did this.
8. MenuGuiImpl - the import java.lang.reflect is not needed
9. ScrollBarGuiImpl - the import com.goldencode.p2j.ui.client.ScrollBar.Position is not needed
10. ScrollBarGuiImpl.getOwningWidget - you need the P2J widget associated with a 4GL widget? If so, why not go up the hierarchy until a parent with a positive widget ID is found... that should be the widget you need. IIRC, there is an API added in trunk which does just this...
11. ToggleBoxGuiImpl - in history entry you have , removed debug code. but there are no changes to reflect this...
12. WindowGuiImpl - you have some spaces added to the first line, please remove them
13. WindowTitleBar.destroy - you are not calling super.destroy() - if you don't need to call it, please add a comment
14. WindowTitleBar.activateTooltips - you are calling TC.activateTooltipForWidget for all caption buttons. But where do you de-activate them? When the title-bar is destroyed, shouldn't these be de-activated? Or is it done automatically by some other code?
15. for the new files, the header first line should be on 98 chars:
    

    ** -#- -I- --Date-- ----------------------------------Description---------------------------------
    

I've rebased the branch. Current revision is 11013.

Igor, I've found that you changed EditorGuiImpl.findMouseSource(), adding this == src.parent(EditorGuiImpl.class) condition there. This led to the fact, that editor scroll bars will be replaced in this method by editor itself. I.e. scrollbars will not be able to process events and show popup menus in this task. Could you please clarify why it is needed?

W/o this fix the selection by mouse drag and scrolling by mouse wheel was not working.

This change to findMouseSource() has been removed in revision 11056 of branch 1811t. It has been replaced with a different approach.

Constantin Asofiei wrote:

Review for 2849a rev 11006:

1. have you tested this with the Web client?

Not yet.

..

1. EditorGuiImpl.mouseClicked - you moved the invalidateSelection(); into the if (@hasFocus()) block - please explain why you did this.

I need that selection will be preserved when user clicks on it with RMB and open editor popup menu with clipboard operations.

..

1. ScrollBarGuiImpl.getOwningWidget - you need the P2J widget associated with a 4GL widget? If so, why not go up the hierarchy until a parent with a positive widget ID is found... that should be the widget you need. IIRC, there is an API added in trunk which does just this...

I didn't know about this possibility. What positive widget ID means?

..

1. WindowTitleBar.activateTooltips - you are calling TC.activateTooltipForWidget for all caption buttons. But where do you de-activate them? When the title-bar is destroyed, shouldn't these be de-activated? Or is it done automatically by some other code?

Yes, this is done automatically in GuiDriver.deregisterWidget()

All other remarks are fixed. Current revision is 11015.

All features are implemented (except icons for window title popup), but the functionality is broken after rebase: When I've committed findMouseSource change editor popup is not shown: mouse source is always ScrollPaneGuiImpl when I click just inside editor.

In previous note please read "When I've commented findMouseSource change" instead of "When I've committed findMouseSource change"

Window title popup isn't working for Web client.. (but it works for SWING). What could be a reason - coordinates? Editor and Scroll popups are working good.

Vadim Gindin wrote:

Constantin Asofiei wrote:

Review for 2849a rev 11006:

1. EditorGuiImpl.mouseClicked - you moved the invalidateSelection(); into the if (@hasFocus()) block - please explain why you did this.

I need that selection will be preserved when user clicks on it with RMB and open editor popup menu with clipboard operations.

OK, but please check this case: EDITOR has some selected text, focus is switched to another widget. After this, RMB is pressed inside the EDITOR: as it doesn't have focus, woudln't mouseClicked():990 if (!hasFocus()) be true? And thus invalidate the selection?

1. ScrollBarGuiImpl.getOwningWidget - you need the P2J widget associated with a 4GL widget? If so, why not go up the hierarchy until a parent with a positive widget ID is found... that should be the widget you need. IIRC, there is an API added in trunk which does just this...

I didn't know about this possibility. What positive widget ID means?

See ThinClient.findLegacySource - this retrieves the P2J widget associated with a legacy 4GL widget (regardless if you click on a scrollbar, for example).

Greg Shah wrote:

Some items in window title bar popup have icons, such as Close, Maximize, Minimize, Restore. Should I implement icon support for menu items now?

No, we will wait on this. Please create a new task.

I need you to finish the current menu tasks which are needed to make menus stable and function properly. That work has been going on for months and we must finish it very quickly.

Have you created a new task for the missing icons feature?

Please also summarize what open issues there are (other than the missing icons).

Greg Shah wrote:

Greg Shah wrote:

Some items in window title bar popup have icons, such as Close, Maximize, Minimize, Restore. Should I implement icon support for menu items now?

No, we will wait on this. Please create a new task.

I need you to finish the current menu tasks which are needed to make menus stable and function properly. That work has been going on for months and we must finish it very quickly.

Have you created a new task for the missing icons feature?

The new task is #3019.

Please also summarize what open issues there are (other than the missing icons).

1. window title popup is not opened for WEB client.
2. Check issues Constantin have noted in the note №117

window title popup is not opened for WEB client.

This is caused by the broken findMouseSource(), right?

Sergey: I assume your fix in 1811t would resolve this. Correct?

Constantin Asofiei wrote:

Vadim Gindin wrote:

Constantin Asofiei wrote:

Review for 2849a rev 11006:

1. EditorGuiImpl.mouseClicked - you moved the invalidateSelection(); into the if (@hasFocus()) block - please explain why you did this.

I need that selection will be preserved when user clicks on it with RMB and open editor popup menu with clipboard operations.

OK, but please check this case: EDITOR has some selected text, focus is switched to another widget. After this, RMB is pressed inside the EDITOR: as it doesn't have focus, woudln't mouseClicked():990 if (!hasFocus()) be true? And thus invalidate the selection?

The problem is in fact that selection is invalidated earlier in this scenario: when EDITOR loses focus. It happen in EditorGuiImpl.processKeyEvent(), line 503. But I've commented this call just for testing purpose and found that you're right. When I click back to EDITOR using RMB - selection is invalidated, that is wrong. I'll remove this call from if (!hasFocus()){ .. } block, but I think some common rule when to call invalidateSelection() is needed..

1. ScrollBarGuiImpl.getOwningWidget - you need the P2J widget associated with a 4GL widget? If so, why not go up the hierarchy until a parent with a positive widget ID is found... that should be the widget you need. IIRC, there is an API added in trunk which does just this...

I didn't know about this possibility. What positive widget ID means?

See ThinClient.findLegacySource - this retrieves the P2J widget associated with a legacy 4GL widget (regardless if you click on a scrollbar, for example).

I think that scroll bar owner is not always widget associated with a legacy 4GL widget: message area can have scroll bars for example. But thanks for advice I'll know about this possibility.

I've committed my current changes and rebased the branch. Now current revision is 11017.

Greg Shah wrote:

window title popup is not opened for WEB client.

This is caused by the broken findMouseSource(), right?

Sergey: I assume your fix in 1811t would resolve this. Correct?

Don't know I should check it with the 2849a branch.

I don't understand.. Sergey, is proposed fix complete? Why wouldn't you commit it?

I don't understand.. Sergey, is proposed fix complete? Why wouldn't you commit it?

Because it contains some changes from 1811t related to the custom cursors. If I will commit all these changes, then there is a chance that they will be committed to the trunc before 1811t. But if you want to restore a title popup menu in the web client, then it is enough to leave the code after rebase and apply only p2j.mouse.js fix. I can commit all these changes or p2j.mouse.js fix only if you agree.

Vadim Gindin wrote:

Window title popup isn't working for Web client.. (but it works for SWING). What could be a reason - coordinates? Editor and Scroll popups are working good.

The reason is p2j.mouse.js in note 124.

Committed revision 11018 fixed p2j.mouse.js.

Vadim Gindin wrote:

I have some difficulty. Lets look at EDITOR that has both scroll bars: horizontal and vertical. I'm confused with widget hierarchy. EditorGuiImpl's first child is ScrollPaneGuiImpl of the same size as Editor itself, i.e. rectangle of editor area including scroll bars areas. Have a look at the image:

I'm not sure it is correct solution. When I click RMB inside EDITOR corresponding findMouseSource() will find noted ScrollPaneGuiImpl instead of Editor itself, that will be processing all events..

Also pay attention on the square in the right bottom corner of EDITOR in a place where scroll bars intersect. This square is probably a part of noted panel and it is insensitive to mouse events: no scroll popup is shown.

Why EditorContent is inherited from AbstractContainer? It does not contain any other widgets. I would suppose to inherit it from AbstractWidget instead. In this case Editor.findMouseSource() will return EditorContent that is more essential and help me to separate cases: when to show editor popup and when to show scroll popup...

What do you think?

In the web client I solved the similar problem, EditorGuiImpl, its scroll pane and its scroll bars are registered as mouse regions with these z-order from high to low values: first scroll bars, then its scroll pane followed by EditorGuiImpl that has the same size as its scroll pane but the lower z-order. If the mouse source are from scroll bars, the source is not modified, but if the mouse source is EditorContent, then it is modified to EditorGuiImpl. In 1811t AbstractContainer.findMouseSource is rolled back, but I think the EditorGuiImpl.findMouseSource is not correct because if the mouse pointer is over the EditorContent, then it returns null. It is my error. I will fix it. Please see WebMouseHandler.raiseMouseEvent in 1811t. The scroll bars and the editor content are identified, but I don't know how about the right bottom corner. I think I can commit the changes from 1811t related to findMouseSource, then scroll bars and the editor content can be separated. And then you will adapt EditorGuiImpl.findMouseSource to your needs. Do you agree?

Don't know how to do it better. In 1811t AbstractContainer.findMouseSource returns the mouse source component. And if EditorGuiImpl.findMouseSource will return the mouse source component, then all other related parts as customs cursors should work properly. Now in 1811t EditorGuiImpl.findMouseSource looks like

   /**
    * Find the widget positioned just bellow the specified mouse position (in physical units).
    * <p>
    * For editors, if the sources is either {@link #editor} or {@link #contentPane}, then return
    * this widget.
    * 
    * @param    p
    *           The mouse physical location.
    * 
    * @return   A widget over which the mouse is positioned or null if none found.
    */

   public Widget<GuiOutputManager> findMouseSource(NativePoint p)
   {
      Widget<GuiOutputManager> src = super.findMouseSource(p);

      return (src == editor || src == contentPane) ? this : src;
   }

I think that 1811t will be committed to the trunc, then the same issues will appear again. It is better to apply AbstractContainer.findMouseSource fix to 2849a branch. The task is 3013.

Ok, If you are sure, I don't mind.

It works fine for SWING, but for WEB it shows editor popup when the user clicks that square in the right bottom corner of editor area..

It works fine for SWING, but for WEB it shows editor popup when the user clicks that square in the right bottom corner of editor area..

I don't know it should help if ScrollPaneGuiImpl.findMouseSource returns null or some marker component that represents that area. WebMouseHandler delivers mouse events and it looks at findMouseSource, but Swing dispatches mouse events differently. I can fix it if you have no objections. Is 2849a ready?

Yes, the branch is ready. Please fix it.

I would propose that editor.physicalBounds().contains(..) should be replaced with editor.getActualBounds().contains(..) because this method also uses screenPhysicalLocation() instead of physicalLocation().

I'm testing my proposal and it probably wrong.. I'm confused with such amount of location methods: location/physicalLocation/screenPhysicalLocation/displayPhysicalLocation/ConfigHelper.getEffectiveLocation/ConfigHelper.getEffectivePixelLocation. Anyway your fix solves the problem. Thank you.

I would propose that editor.physicalBounds().contains(..) should be replaced with editor.getActualBounds().contains(..) because this method also uses screenPhysicalLocation() instead of physicalLocation().

I think we need to check these methods thoroughly, because the mouse enter/exit events are also based on getActualBounds(). Because if these functions will be changed, then all code based on the current state will be broken.

Does this check if ( this.getActualBounds().contains(e.getX(), e.getY()) ) work properly? Theoretically it should return true if the mouse pointer is inside the context editor.

Colleagues. I have a question. At the any time there is only one opened popup menu is possible in the application. Should I fix it now or it is better to wait when popup menus will be implemented in separate window?

If I should fix it now - how to do it better?

1. I would propose to add MENU reference to some field ThinClient.menu and set it each time popup is opened and correspondingly close previously opened popup menu.
2. Another way: when popup is opened add some mouse listener that will listen all mouse clicks (regardless of event source) and hide opened popup menu. As I thought there was such listenere, but I couldn't find it.

Sergey Ivanovskiy wrote:

Does this check if ( this.getActualBounds().contains(e.getX(), e.getY()) ) work properly? Theoretically it should return true if the mouse pointer is inside the context editor.

Yes, it works: mouse pointer is inside EDITOR inner area (excluding scrollbars).

At the any time there is only one opened popup menu is possible in the application. Should I fix it now or it is better to wait when popup menus will be implemented in separate window?

Yes, I think it can be fixed now.

If I should fix it now - how to do it better?

1. I would propose to add MENU reference to some field ThinClient.menu and set it each time popup is opened and correspondingly close previously opened popup menu.
2. Another way: when popup is opened add some mouse listener that will listen all mouse clicks (regardless of event source) and hide opened popup menu. As I thought there was such listenere, but I couldn't find it.

Does this reference need to be in ThinClient? I'd prefer if this was a static member of the popup menu itself and have all the logic hidden inside that class.

Greg Shah wrote:

Does this reference need to be in ThinClient? I'd prefer if this was a static member of the popup menu itself and have all the logic hidden inside that class.

OK, I've fixed it by adding static variable to MenuGuiImpl. So there is no opened issues in this task. Please review. revision 11022.

• Menu.java - copyright date needs to be set to 2015-2016
• Menu.showPopupMenu - evt parameter needs javadoc
• OutputManager:1289 - uncomment the line
• DialogBoxWindow - missing history entry
• EditorPopupGuiImpl.createEditorPopupMenu - you are simulating the Cut/Copy/Paste/Undo actions by posting actual keys (i.e VK_CTRL_Z), which might invoke any registered triggers, in P2J. Please check if 4GL raises triggers when these are executed via popup menu and also what happens in P2J
• WindowTitleBar - you have two history entry numbers/dates, merge them.
• Uncomment the lines from EmulatedWindowState

• you have changes for Swing driver, but are these issues present in the Web driver, too, or were they not present?
• is this code only trying to solve an issue with the Editor? Or with some other widgets, too?
• I think now is the time to finish the work findMouseSource, related to the TODO: move this widget-specific logic into methods that are implemented in the widget hierarchy so that we do not have direct references to widget classes in this common code.
• lines like this:
  

              if (lastHoveredWidget != null && !lastHoveredWidget.getActualBounds().contains(e.getX(), e.getY()))
              if (lastHoveredWidget != mouseSource
                       && mouseSource!= null && mouseSource.getActualBounds().contains(e.getX(), e.getY()))
  

  
  need to be formatted like this:
  

              if (lastHoveredWidget != null && 
                  !lastHoveredWidget.getActualBounds().contains(e.getX(), e.getY()))
              if (lastHoveredWidget != mouseSource && 
                  mouseSource!= null               && 
                  mouseSource.getActualBounds().contains(e.getX(), e.getY()))
  

Constantin Asofiei wrote:

Sergey, about the changes in the mouse handler classes:

• you have changes for Swing driver, but are these issues present in the Web driver, too, or were they not present?

The issues from 3013 are not reproduced for the Web client

• is this code only trying to solve an issue with the Editor? Or with some other widgets, too?

These changes impacted all other widgets and fixed the 3013 issues.

• I think now is the time to finish the work findMouseSource, related to the TODO: move this widget-specific logic into methods that are implemented in the widget hierarchy so that we do not have direct references to widget classes in this common code.

Please look at #3013-53, this task was postponed.

Constantin Asofiei wrote:

Vadim, about your changes in 2849a rev 11022:

• Menu.java - copyright date needs to be set to 2015-2016
• Menu.showPopupMenu - evt parameter needs javadoc
• OutputManager:1289 - uncomment the line
• DialogBoxWindow - missing history entry
• EditorPopupGuiImpl.createEditorPopupMenu - you are simulating the Cut/Copy/Paste/Undo actions by posting actual keys (i.e VK_CTRL_Z), which might invoke any registered triggers, in P2J. Please check if 4GL raises triggers when these are executed via popup menu and also what happens in P2J
• WindowTitleBar - you have two history entry numbers/dates, merge them.
• Uncomment the lines from EmulatedWindowState

I've fixed code cleanup remarks and checked key triggers. Behaviour is the same: when menu-item is clicked - corresponding key trigger is not being executed. Testing procedure is gui_editor_key_triggers_mes.p. Current branch revision is 11023.

Are there some other remarks?

If there are no new remarks may be I could run regression testing?

Vadim, please fix the formatting issues in SwingMouseHandler (from note 149) too, rebase to trunk rev 10983 and start runtime testing.

Also, please check test 454, too, and see how it behaves.

What is a "test 454"?

Current branch version 11026 (rebased and fixed SwingMouseHandler). Here are failed tests: gso_190, tc_job_002, tc_job_matlcron_002, tc_codes_employees_017.

I've checked gso_190 and tc_codes_employees_017 manually. They PASSED. Can I commit the changes?

Colleagues, after several starts the customer's Gui Harness became working similar to it works for trunk. It happen gradually. First starts were not showing even the second window. After several starts second window appeared but the third window wasn't visible (after clicking 'OK'). Finally after another several starts the third window and others became visible/working. It seems it were something like db caching issues: cache were filled step by step with each run and finally starts working effectively (for concrete machine). It's just a hypothesis.

I've rebased the branch with trunk revision 10985. Current branch revision is 11028. I've checked 454 test - there are some exceptions are met (like it is for trunk).

Vadim Gindin wrote:

I've rebased the branch with trunk revision 10985. Current branch revision is 11028. I've checked 454 test - there are some exceptions are met (like it is for trunk).

Have you checked to system popup-menus for widgets in test 454 screen?

Before rebasing it wokred, but after it - it stopped working. I'm going to fix it.

Window title popup works, but editor and scroll popups don't work. I've checked simple scenario like gui_editor_popup.p - all popup are working. But for 454 test only window title popup is shown. I've tried to debug it using remote debugging: mouseClicked() methods of EditorGuiImpl and ScrollPopupGuiImpl are not called. I would suppose that the reason could be in findMouseSource() methods or wrong coordinates. What do you think?

Vadim Gindin wrote:

I would suppose that the reason could be in findMouseSource() methods or wrong coordinates. What do you think?

Please debug SwingMouseHandler.mouseClicked and ThinClient.processProgressEvent:15461 (line if (event instanceof MouseEvt)) and see what the window.findMouseSource call returns.

Fonts probably not only bigger but different: we can see it when comparing 's', 'A' and 't' characters.

Strange.. SCROLL_UP/SCROLL_DOWN/PAGE_DOWN/PAGE_UP work well, but others do not. For example simplest operation TOP. Why it is simplest - because it only calls scrollBar.notifyScrollListeners(0); where 0 - is the first possible bar position. In other words even if somewhere position could be calculated incorrectly (just hypothesis) - but not here. The first position is always exists.

Vadim Gindin wrote:

Fonts probably not only bigger but different: we can see it when comparing 's', 'A' and 't' characters.

Please check which OS font is Windows using for the OS popups (and for the other, custom popups, too). We need to use the same font for the OS popups. Also, I don't recall, can the font for the custom popups be set via some FONT attribute?

Vadim Gindin wrote:

I would suppose, that such bug are somehow related with scroll bar implementation. What do you think? Do you insist that I would fix it?

What bug are you referring to? The one in note 165?

Constantin Asofiei wrote:

Vadim Gindin wrote:

I would suppose, that such bug are somehow related with scroll bar implementation. What do you think? Do you insist that I would fix it?

What bug are you referring to? The one in note 165?

Yes, that one.

..

MENU, MENU-ITEM do not suport FONT attribute. By the way how to find out what font is used in popup menus?

Vadim Gindin wrote:

Constantin Asofiei wrote:

Vadim Gindin wrote:

I would suppose, that such bug are somehow related with scroll bar implementation. What do you think? Do you insist that I would fix it?

What bug are you referring to? The one in note 165?

Yes, that one.

Do you have standalone tests for frame scrollbars, to check the OS popup? If not, please create one.

..

MENU, MENU-ITEM do not suport FONT attribute.

I think you've mentioned before, thanks for remaining me.

By the way how to find out what font is used in popup menus?

On windev01, try to change the font in the Windows Appeareance/Color Scheme dialog and find which one is configured for OS the popup-menu.

I've tested menu-item font for editor popups in a standalone test and in the customer's 454 test. Here is the result:
1. Standalone test: font=Microsoft Sans Serif, fontHeight=13
2. The customer's GUI app 454 test: font=System, fontHeight=16

I've tested popup fonts on windev01. The same font is used for all menus regardless of it is system popup or regular.

I've noticed that fonts are different also in the first window: Window title and Button label have the same font (probably Sans Serif) and "select" widget has another font (looks like that very "System" font and size). So it also looks like the common problem for customer's Test Harness.

Vadim Gindin wrote:

So you probably would want me to create standalone test for similar BROWSE (not FRAME, as you wrote) scrollbar - to reproduce that error. Isn't it?

Exactly, make sure you are testing for each widget which can have scrollbars (selection-list, combo-box in "list" mode, etc).

Vadim Gindin wrote:

I've tested menu-item font for editor popups in a standalone test and in the customer's 454 test. Here is the result:

Please check on windev01 which fonts are used for popups. There might be some setting/special font, and we will need to hardcode to that font, if so.

Vadim Gindin wrote:

I've used one of existing browse tests: browse/gui/brws-hscoll-col1.p

Scrolling fails. See attached gif.

Pay attention also at initial wrong calculation of bar size.

Should I fix it?

Is this a regression in trunk? I am wondering if this was caused with one of my changes to frame layout.

It happen in 2849a. I didn't check it for trunk. By the way, Hynek, does the BrowseGuiImpl support "scroll here" operation, i.e. content scrolling to random position?

Vadim Gindin wrote:

It happen in 2849a. I didn't check it for trunk. By the way, Hynek, does the BrowseGuiImpl support "scroll here" operation, i.e. content scrolling to random position?

You might want to check Browse.scrollToRow().

It happen in 2849a. I didn't check it for trunk.

Please check it. If it is in the trunk, then it is not something to be fixed in this task. I really want to get this task tested and merged into the trunk ASAP. We only want to ensure that it causes no regressions.

Hynek, could you have a look at ScrollBar.onButtonClick(..). There is a comment "non-trimmed into [0, max] values.". Certainly, why position is not trimmed before call constroller.positionUpdated(..)?

What should I pass to this method for operations TOP/BOTTOM? I've tried 0/max - but that was wrong.
What should I pass to this method for operation SCROLL_HERE?

Greg Shah wrote:

It happen in 2849a. I didn't check it for trunk.

Please check it. If it is in the trunk, then it is not something to be fixed in this task. I really want to get this task tested and merged into the trunk ASAP. We only want to ensure that it causes no regressions.

I've checked the trunk. Yes this error (wrong initial calculation of scroll bar size) is also exists in trunk. Thank you.

Vadim Gindin wrote:

Hynek, could you have a look at ScrollBar.onButtonClick(..). There is a comment "non-trimmed into [0, max] values.". Certainly, why position is not trimmed before call constroller.positionUpdated(..)?

What should I pass to this method for operations TOP/BOTTOM? I've tried 0/max - but that was wrong.
What should I pass to this method for operation SCROLL_HERE?

First of all you should figure out how the native 4GL browse widget works. The browse is paged when the query returns many entries. So make sure you try one query to fit the result into one page, and one query to have the results paged.

For the TOP/BOTTOM I would think the scroll row values would be 0/max items count (BrowseModel.getRowCount()).

For SCROLL_HERE I am not sure, you will have to experiment.

My question rather was about API..

Vadim Gindin wrote:

My question rather was about API..

I am afraid there is no simple API to support TOP/BOTTOM/SCROLL_HERE directly yet.

I need to implement the following operations: TOP/BOTTOM/SCROLL_HERE

Vadim, for top and bottom please refactor the code in Browse.onKeyPressed which handles

if (key == Keyboard.KA_HOME ||
    key == Keyboard.KA_LEFT_END)

if (key == Keyboard.KA_END ||
    key == Keyboard.KA_RIGHT_END)

For SCROLL_HERE use BrowseGuiImpl.verticalScroller.positionUpdated. I hope that position parameter of the thumb matches the popup vertical position. Otherwise there are other ScrollBarController functions like getThumbSize() which can help you if you need some kind of position offset.

Thanks for advice. I've fixed browse implementation and rebased the branch 2849a using current trunk revision 10986. I've committed changes with branch revision 11031. Please review.

If it works properly then I'm fine with it.

If there are no remarks - can I run testing?

1. Scrollbar.java: code like this
   

         if (ScrollStep.SHORT.equals(scrollStep))
            step = shortStep;
         if (ScrollStep.LONG.equals(scrollStep))
            step = longStep;
   

   
   needs to be formatted like this:
   

         if (ScrollStep.SHORT.equals(scrollStep))
         {
            step = shortStep;
         }
         if (ScrollStep.LONG.equals(scrollStep))
         {
            step = longStep;
         }
   

2. ScrollPopupGuiImpl: you have a import org.hibernate - please remove it

Vadim Gindin wrote:

If there are no remarks - can I run testing?

Yes, you can go ahead and test. I assume the issues we've discussed about testing the OS popup with scrollable widgets (editor, selection-list, drop-down, etc) is done? In note 17 you mentioned that there is no OS popup for some widgets, what I'm referring is checking the scrollbar OS popup.

PS: don't forget to rebase

Regression testing has finished at the stage of CTRL+C tests. Here are the failures:

   4 ctrlc_11_session1 CTRL-C 11 (Session 1) - CL - GSO - RFQ. FAILED CONCURRENT BACKOUT Driver 0 349.176         
failure in step 18: 'Timeout while waiting for event semaphore to be posted.'                                     

   5 ctrlc_11_session3 CTRL-C 11 (Session 3) - CL - GSO - RFQ. FAILED CONCURRENT BACKOUT Driver 1 287.445         
failure in step 11: 'timeout before the specific screen buffer became available (Mismatched data at line 0, column
 10. Expected ' ' (0x0020 at relative Y 0, relative X 10) and found 't' (0x0074 at relative Y 0, relative X 10), t
emplate: screens/common/service_ordering_fresh.txt.)'                                                             

   6 ctrlc_11_session4 CTRL-C 11 (Session 4) - CL - GSO - RFQ. FAILED CONCURRENT BACKOUT Driver 2 300.001         
failure in step 1: 'Timeout while waiting for event semaphore to be posted.'  

It seems that these errors could be related with some sided reason. Am I right?

Is it normal when main part wasn't tested if CTRL+C failures are met?

Vadim Gindin wrote:

Regression testing has finished at the stage of CTRL+C tests. Here are the failures:
[...]

It seems that these errors could be related with some sided reason. Am I right?

Yes, those timeouts are probably not related to your changes.

Is it normal when main part wasn't tested if CTRL+C failures are met?

What do you mean by "wasn't tested"? Are there any errors in longs?

Constantin Asofiei wrote:

Vadim Gindin wrote:

Regression testing has finished at the stage of CTRL+C tests. Here are the failures:
[...]

It seems that these errors could be related with some sided reason. Am I right?

Yes, those timeouts are probably not related to your changes.

OK, I've rerun regression testing.

Is it normal when main part wasn't tested if CTRL+C failures are met?

What do you mean by "wasn't tested"? Are there any errors in longs?

I wanted to say "wasn't ran" - testing ended up with CTRL+C failures.

Vadim Gindin wrote:

Is it normal when main part wasn't tested if CTRL+C failures are met?

What do you mean by "wasn't tested"? Are there any errors in longs?

I wanted to say "wasn't ran" - testing ended up with CTRL+C failures.

Do you have the ~/testing/testing_yyyymmdd_hhmmss.log file for this failed run? Because the main part should always start if runtime-testing is used, even if CTRL-C fails (unless there is some other failure which aborts the scripts).

Yes testing_* log file exists. Here is a corresponding part of it:

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]  
     [exec] Running test plan... Completed in 1 hours, 7 minutes and 55.815 seconds. Status = FAILED.
     [exec] Generating HTML reports... Completed in 0.511 seconds.
     [exec] ** CTRL-C part has failed!
     [exec] ** Running main part...
     [exec] Main part can not be ran in the 23:45 - 01:59 interval!
     [exec] ** MAIN part has failed!

Testing ended too early (1.07 hrs) and "results" folder both contains only results of CTRL+C tests.

Oh.. now I see the reason:

     [exec] Main part can not be ran in the 23:45 - 01:59 interval!

Here are failed tests:
tc_job_002
tc_job_clock_001
tc_job_clock_002
tc_job_clock_004
tc_job_clock_005
tc_pay_sfc_003

The last one passed single testing. So we can assume the main part is passed testing.

CTRL+C:
ctrlc_11_session1
ctrlc_11_session3
ctrlc_11_session4

What to do with CTRL+C failures? Can really these tests fail because of my UI changes?

Thanks. Yes, it passed. I've rebased to current trunk revision. Current branch revision now is 11035.

Since the rebase required a non-trivial merge of the changes, please run main regression testing 1 more time. Do not worry about the CTRL-C testing.

In addition: have you run the standalone GUI tests recently?

Greg Shah wrote:

Since the rebase required a non-trivial merge of the changes, please run main regression testing 1 more time. Do not worry about the CTRL-C testing.

In addition: have you run the standalone GUI tests recently?

Yes, but not after merge. I'll test them again and if all is OK I'll run regression testing.

Very good. If it passes all of that testing, you can merge to trunk.

I've found a bug: when I click RMB in FillIn with some text - it all become selected whenever previous selection was. It because of FOCUS_GAINED listener in FillInGuiImpl. It is called several times.. I'm digging in that.

ThinClient.waitForWorker() is running 3 times. How is that possible!?

Vadim Gindin wrote:

ThinClient.waitForWorker() is running 3 times. How is that possible!?

Please explain the scenario where this happens.

Here is the test:

def var ch1 as character.
def button btn-exit label "Exit".
def frame frame1 ch1 skip btn-exit with side-labels centered.

enable all with frame frame1.

on entry of ch1 do:
   message "ENTRY ch1".
end.

on entry of frame frame1 do:
   message "ENTRY frame1".
end.

wait-for choose of btn-exit.

There is only one FillIn field. When I've entered for example string "abcdef" and selected only "ef" I press RMB. As a result of RMB: editor system popup is opened and all characters "abcdef" become selected in the field. I.e. I can't CUT some part of string..

I've debugged that and found that focusManager.adjustFocus(); is called 3 times in ThinClient.waitForWorker(..) line 11291. That leads to 3 FocusEvent.FOCUS_GAINED and FillInGuiImpl.onFocusGained() listener changes selection at third call.

Vadim Gindin wrote:

I've debugged that and found that focusManager.adjustFocus(); is called 3 times in ThinClient.waitForWorker(..) line 11291. That leads to 3 FocusEvent.FOCUS_GAINED and FillInGuiImpl.onFocusGained() listener changes selection at third call.

When ThinClient.waitForWorker(..) line 11291 reaches execution, it means there is an event posted to the event queue. In these 3 cases, which events are returned by the ThinClient.waitForWorker(..) line 11166 code event = waitForEvent(seconds, frameAsTarget, true, true);? Are these 3 separate mouse events (mouse down, mouse click, mouse up)?

Yes, 3 different events: PRESSED, RELEASED, CLICKED. In other words, during one action 3 events are processed and FOCUS_GAINED listener is called 3 times. Thanks. I got it.

Vadim Gindin wrote:

Yes, 3 different events: PRESSED, RELEASED, CLICKED. In other words, during one action 3 events are processed and FOCUS_GAINED listener is called 3 times. Thanks. I got it.

OK, please test which events (and which mouse button) can cause a focus-gain on a widget; after this, limit the focus gain on only those cases.

I'd fixed FillIn focus bugs. Please review. Bzr revision (11039) - already rebased.

I've added system popups support to OverlayWindow, but it is cut by overlay window size. That will be unactual when menu implementation will be moved to a separate window.

Vadim Gindin wrote:

I'd fixed FillIn focus bugs. Please review. Bzr revision (11039) - already rebased.

I'm OK with the changes in rev 11039. You need to rebase again.

Anything else to do on this task?

Does this level of code pass standalone GUI regression testing?

Constantin, here are the following founded functionality:
1. I've found that if I'm getting in editor popup right from Editor inner area or FillIn the cursor TEXT is remained in popup the same, but in Progress it is DEFAULT pointer wherever I come in.
2. LOAD-MOUSE-POINTER is supported by menus too. I.e. it probably possible to set custom mouse pointer for menus (need testing).

I'm working on it. I don't know: include that in this task or in some other.

Greg, what do you mean "Standalone"? Automated testing wasn't ran.

Please create a separate task for the pointer issues. Both issues can be in the same task.

Greg, what do you mean "Standalone"? Automated testing wasn't ran.

This is the "standalone GUI tests" we discussed in notes 201 and 202.

I didn't understand you right thereat. Sorry.

I only ran 454 test scenario at some moment. Could you clarify what scenarios should I check?

For the issues pointed in the note №215 the new task #3056 is created.

The branch 2849a is rebased to current trunk revision. Current branch revision is 11041.

Standalone GUI Test List

./ask-gui.p
./hello.p
./demo/calc.p
./demo/calc-static.p
./demo/calc-static-chars.p
./demo/calc-static-pixels.p
./toggle_box/gui/tbx_present.p
./combo_box/combo_box9_1.p
./rectangle/rect_test2.p
./rectangle/rect_test6.p
./rectangle/rect_test7_1.p
./image/image0.p
./image/image10_1.p
./button/gui_btn_test3.p
./button/gui_btn_test4.p
./button/gui_btn_test5.p
./demo/movie-ratings-dynamic.p
./demo/movie-ratings-static.p
./demo/simple_windows.p
./demo/demo_widgets.p
./window_sizing/create_empty_window.p
./window_sizing/default_empty_window.p
./window_sizing/test_runner.p
./simple_alert_box.p (don't test, this is known to be broken)
./simpler_alert_box.p (don't test, this is known to be broken)
./message-update6.p
./minimal_view_dynamic_widget.p (don't test, this is known to be broken)
./menu/simple_sm.p
./menu/popup_ext.p
./window_parenting/waitfor_2wnd.p
./frame-z-order/zw1.p
./browse/gui/browse-gui-stat1.p
./mouse/load-mouse-pointer.p (see mouse/mouse-cursor-test-plan.txt for details)

I should check them manually in my machine for the current branch (2849a), right?

I should check them manually in my machine for the current branch (2849a), right?

Right.

During the testing of ./demo/demo_widgets.p there is exception happen from time to time when choosing values in the combo-box (not each selection):

Apr 05, 2016 9:54:59 PM Dispatcher.processInbound() 
SEVERE: {main} 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:705)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:319)
    at com.goldencode.p2j.net.Conversation.waitMessage(Conversation.java:257)
    at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1129)
    at com.goldencode.p2j.net.Queue.transact(Queue.java:586)
    at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:223)
    at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:163)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1425)
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:97)
    at com.sun.proxy.$Proxy4.standardEntry(Unknown Source)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:294)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:104)
    at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:202)
    at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:396)
    at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:1)
    at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:265)
Caused by: java.lang.IllegalStateException: widget is not in the container
    at com.goldencode.p2j.ui.client.widget.AbstractWidget.screenPhysicalLocation(AbstractWidget.java:849)
    at com.goldencode.p2j.ui.client.widget.AbstractWidget.getActualBounds(AbstractWidget.java:2327)
    at com.goldencode.p2j.ui.client.gui.driver.swing.SwingMouseHandler$9.run(SwingMouseHandler.java:289)
    at com.goldencode.p2j.ui.chui.ThinClient.waitForEvent(ThinClient.java:13219)
    at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:11168)
    at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10742)
    at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10696)
    ... 21 more

END DRAWING CYCLE FOR WINDOW 1

./window_sizing/create_empty_window.p open and close windows nonstop. Logs aren't contain exception, just some warnings

Vadim Gindin wrote:

During the testing of ./demo/demo_widgets.p there is exception happen from time to time when choosing values in the combo-box (not each selection):
[...]
There are also some visual bugs: when I open combo-box chcb in frame f2 the other combo-box chcb in frame f1 is also opened and right after that closed.

The exception you found is related to my code changes, but for the current 1811t there are no exceptions with IllegalStateException: widget is not in the container for demo_widgets.p and the combobox bug isn't reproduced. Should I take this bug for 2849 branch?

Sergey, probably no. I'm just testing my changes.

I'm continuing: other window_size/ test are executed in the same way as described in the note 222.

message_update6.p in GUI doesn't show "You entered" message. There is MESSAGE .. UPDATE statement. May be I just forgot what to press in GUI. In CHUI I just press Enter.

./menu/simple_sm.p and ./menu/popup_ext.p works fine except "Extra popup" bug defined in #2911 which is waiting for moving menus to separate window implementation.

Vadim Gindin wrote:

./window_sizing/create_empty_window.p open and close windows nonstop. Logs aren't contain exception, just some warnings

Check both server and client logs. Debug the client and add RuntimeException brake point to brake when the exception is caught and uncaught.

For the window_sizing tests, run them in the task branch and trunk and compare logs (window_sizing.log). If they are equal the tests passed. Also you should not see any assertion alert boxes.

Hynek thanks. I've checked it. Logs are similiar. RuntimeException breakpoint wasn't activated. trunk works in the same way.

I've finished testing. I didn't find other errors in remaining tests. I'm not sure I've executed mouse pointer test correctly, but as I can understand it works correctly.

Can I run regression testing?

During the testing of ./demo/demo_widgets.p there is exception happen from time to time when choosing values in the combo-box (not each selection):
[...]
There are also some visual bugs: when I open combo-box chcb in frame f2 the other combo-box chcb in frame f1 is also opened and right after that closed.

The exception you found is related to my code changes, but for the current 1811t there are no exceptions with IllegalStateException: widget is not in the container for demo_widgets.p and the combobox bug isn't reproduced. Should I take this bug for 2849 branch?

Sergey: Do you think this is a real issue or is it just related to the fact that the fix you put in is partial?

Don't know because this bug isn't reproduced for 1811t. I am checking 2849 and 1811t differences related to my changes but it is not obvious.
Found ThinClient differences, 2849 branch wnd.repaint() but 1811t w.repaint() inside independentEventDrawingBracket(w, new Runnable()

   @Override
   public Boolean moveToBottom(int id)
   {
      Widget<?> w = getWidget(id);
      if (w == null)
      {
         return null;
      }

      if (w instanceof TopLevelWindow)
      {
         WindowManager.moveToBottom((TopLevelWindow)w);
         return Boolean.TRUE;
      }

      @SuppressWarnings("unchecked")
      Frame<?> frame = (Frame<?>) UiUtils.locateFrame(w);
      if (frame == null)
      {
         return Boolean.FALSE;
      }

      final TopLevelWindow wnd = frame.topLevelWindow();
      if (wnd == null)
      {
         return Boolean.FALSE; 
      }

      boolean moved = w.moveToBottom();
      independentEventDrawingBracket(w, new Runnable()
      {
         @Override
         public void run()
         {
            wnd.repaint();
         }
      });

      return Boolean.valueOf(moved);
   }

moveToTop the same code changes 2849a wnd.repaint(), but 1811t w.repaint().

Constantin, could you help 1811t is rebased with w.repaint() but P2j trunc have this version wnd.repaint() for methods public Boolean moveToBottom(int id) and
public Boolean moveToTop(int id)?

Sergey Ivanovskiy wrote:

Constantin, could you help 1811t is rebased with w.repaint() but P2j trunc have this version wnd.repaint() for methods public Boolean moveToBottom(int id) and
public Boolean moveToTop(int id)?

The correct version is in 1811t - the Runnable instance repaints the widget, not the full window.

Thank you, the changes between 1811t and 2849a related to the particular fix for cursor styles if the mouse pointer is over a target widget looks the same but there are big differences that at first glance are unrelated but can produce this bug.

OK, then we will not worry about this bug here.

Vadim: you can go ahead and test. But please note that I plan to get 1811t tested and merged to trunk first. However, if 2849a passes regression testing AND there are issues with 1811t, then you will be allowed to go first.

Here is the failures list:
gso_tests (1)
gso_190

tc_tests (10)
tc_codes_employees_017
tc_codes_employees_019
tc_codes_employees_021
tc_codes_employees_024
tc_job_002
tc_job_clock_001
tc_job_clock_002
tc_job_clock_004
tc_job_clock_005
tc_item_master_104
gso_rfq_tests (2)
login_to_main_rfq_menu
logout_from_main_rfq_menu

During single testing the following tests from noted above list were failed:
tc_item_master_104
login_to_main_rfq_menu
logout_from_main_rfq_menu

Talking about logout_from_main_rfq_menu failed test, there is a different window title that is assumed as error:
Log file:

┌────────────────────── TIMCO - Greensboro GSO TIPR833K ───────────────────────┐

and the sample from screens/navigation/syman_rfq_menu.txt:

┌─────────────────── TIMCO - Greensboro GSO TIPR833K - RFQ ────────────────────┐

The error itself (1st step):

   FAILED 180.070                                                                                         
timeout before the specific screen buffer became available (Mismatched data at line 1, column 20. Expected
 ' ' (0x0020 at relative Y 1, relative X 20) and found '─' (0x2500 at relative Y 1, relative X 20), templa
te: screens/navigation/syman_rfq_menu.txt.)   

Is it known bug?

Talking about login_to_main_rfq_menu there is just timeout error at the second step:

   1 RUN-CODE                                                                                             

   PASSED 41.803                                                                                          

   2 WAIT-FOR-SCREEN-BUFFER   
millis = ʼ180000ʼ; value = ʼ@devsrv01:~$ʼ failing screen =                                                
 MAJIC                             Syman Menu                    04/06/16 07:32                           
┌────────────────────── TIMCO - Greensboro GSO TIPR833K ───────────────────────┐                          
│ I  Inventory                           F  Syman - Master File Maintenance    │                          
│ O  Order Entry                         C  Codes and Parameters               │                          
│ V  Service Order Entry                 M  Menu of Outputs and Reports        │                          
│ P  Purchasing                          U  Utilities                          │                          
│ G  Shipping                            N  Inquiry Menu                       │                          
│ D  Data Collection                                                           │                          
│                                        E  Estimating                         │                          
│ 1  General Ledger                      J  Job Definition                     │                          
│ 2  Accounts Receivable                 A  Component Tracking                 │                          
│ 3  Accounts Payable                    W  Work Center Capacity & Dispatching │                          
│ 4  Payroll                                                                   │                          
│ 5  Fixed Assets                        R  Quit MAJIC/Symix                   │                          
│ 6  Customer Menu                                                             │                          
│ 7  RFQ Menu                                                                  │                          
│ 8  Quality Assurance                                                         │                          
│                                                                              │                          
└──────────────────────────────[     No mail    ]──────────────────────────────┘                          
   [F1]-Launch [?]-Browse [F2]-Help                                                                       
   Function: payroll                            Menu: symix                                               

   FAILED 180.050                                                                                         
timeout before the specific screen buffer became available                                                

Is it also known bug? What it can be related with?

I'm trying to reproduce the failure of the test tc_item_master_104. What does the following step mean:

   44 RELEASE-MUTEX                                                                                       
name = ʼcycle_countingʼ;   

It means that it some other test is waiting on a mutex semaphore named "cycle_counting". This test is releasing the semaphore, allowing that other test to continue.

Probably it is not possible to reproduce this test manually. Isn't it?
The next step in this test FAILED:

   44 RELEASE-MUTEX                                                                                       
name = ʼcycle_countingʼ;                                                                                  

   PASSED 0.000                                                                                           

   45 TEXT-FILE-COMPARISON                                                                                
baseline = ʼreports/tc_item_master_104.txtʼ; actual = ʼtc_item_master_104.txtʼ; downloadDir = ʼresults/201

60406_072811/downloads/ʼ; download = true; excludes = [each page: row 0 (columns 113 - 132), page 1552: ro
w 13 (columns 23 - 23), page 1552: row 15 (columns 23 - 23)];                                             

   FAILED 0.998                                                                                           
Mismatched data at absolute row 51993, relative row 25, page # 897, page size 58, last page false, column 
index 16. Expected ' ' (0x20) and found '0' (0x30).   

Just re-run the entire "main" set.

I've tried to run run_main.sh script, but it fails:

   1 prepare_tests Specific scripts to prapare DB to test execution. FAILED PRE_CONDITION                 
   prepare_tests 233.282                                                                                  
failure in step 8: 'timeout before the specific screen buffer became available (Mismatched data at line 3,
 column 13. Expected ' ' (0x0020 at relative Y 3, relative X 13) and found 's' (0x0073 at relative Y 3, re
lative X 13), template: screens/init/codes_buyer/syman_add_step2.txt.)'                 

What I've missed?

Vadim Gindin wrote:

I've tried to run run_main.sh script, but it fails:
[...]

What I've missed?

Have you restored the DB? Why not use run_regression.sh main-regression instead of running the run_main.sh script explicitly?

Thanks Greg. I didn't know about this possibility. OK. I've executed the second regression testing and those results do not intersect previous. So we can assume that testing passed.

OK, very good. We have some other branches that need to be merged to trunk first. I will let you know when to rebase. You may have to run another round of automated regression testing at that time. There is nothing else to do on this task for now.

I've rebased the branch to current trunk revision (11000). Current branch revision is 11049.

I've got testing results. Here are the failed tests:

 gso_278, 
 tc_codes_employees_019
 tc_codes_employees_024
 tc_job_002
 tc_item_master_007

 gso_278
 tc_item_master_007

Are you saying that the gso_278 and tc_item_master_007 consistently fail (are real regressions)?

Greg Shah wrote:

Are you saying that the gso_278 and tc_item_master_007 consistently fail (are real regressions)?

I'm not sure because tc_item_master_007 also contains the step:

   14 ACQUIRE-MUTEX                                                                                                    
name = ʼcycle_countingʼ;                                                                                               

gso_278:
There are some sftp steps.. This scenario also leads to "Cycle Counting" page, I'd met before:

   57 CHECK-SCREEN-BUFFER                                                                                              
wait = true; millis = ʼ180000ʼ; failing screen =                                                                       
04/12/2016                  CYCLE COUNT UPDATE FOR GSO                  06:16:35                                       
  ┌─────────────────────────────────────────────────────────────────────────┐                                          
  │Item            Fac / Loc            Turnarnd S  Cut Off Qty    Count Qty│                                          
  │--------------- -------------------- -------- - ------------ ------------│                                          
  │UA001608826     H1   A-016           0001BVFX N        2.000            ?│                                          
  │UA001610481     H1           ┌SITE SELECTION┐ N        0.000            ?│                                          
  │UA001610481     H1           │Site: GSO     │ N        2.000            ?│                                          
  │UA002800596     H1           └──────────────┘ N        2.000            ?│                                          
  │UA004420006     H1                   0001H31F N        1.000            ?│                                          
  │UA004420006     H1                   0001HEG7 N        1.000            ?│                                          
  │UA004810462     H1                   0001H31J N        1.000            ?│                                          
  │UA005600074     H1                            N        0.000            ?│                                          
  │UA005631601     H1                            N        0.000            ?│                                          
  │UA005631601     H1                   0001J39S N        1.000            ?│                                          
  │UA005810023     H1                   0001HZNE N        2.000            ?│    
  │UA005810023     H1                   0001DTRR N        1.000            ?│                                          
  │UA007514142     H1                   0001c3e9 N        1.000            ?│                                          
  │UA007514142     H1                   0001c8v3 N        2.000            ?│                                          
  │UA007621075     H1                   0001HA25 N        8.000            ?│                                          
  │UA007621075     H1                   0001HD9L N       12.000            ?│                                          
  └─────────────────────────────────────────────────────────────────────────┘                                          
Enter Site to Work or ? for browser                                                                                    

   FAILED 180.069                                                                                                      
timeout before the specific screen buffer became available (Mismatched data at line 14, column 44. Expected 'D' (0x0044
 at relative Y 14, relative X 44) and found 'H' (0x0048 at relative Y 14, relative X 44), template: screens/gso/gso_278
/cycle_count_generation_step16.txt.)                                                                                   

10/05/2009                  CYCLE COUNT UPDATE FOR GSO                  20:15:48
  ┌─────────────────────────────────────────────────────────────────────────┐
  │Item            Fac / Loc            Turnarnd S  Cut Off Qty    Count Qty│
  │--------------- -------------------- -------- - ------------ ------------│
  │UA001608826     H1   A-016           0001BVFX N        2.000            ?│
  │UA001610481     H1           ┌SITE SELECTION┐ N        0.000            ?│
  │UA001610481     H1           │Site: GSO     │ N        2.000            ?│
  │UA002800596     H1           └──────────────┘ N        2.000            ?│
  │UA004420006     H1                   0001H31F N        1.000            ?│
  │UA004420006     H1                   0001HEG7 N        1.000            ?│
  │UA004810462     H1                   0001H31J N        1.000            ?│
  │UA005600074     H1                            N        0.000            ?│
  │UA005631601     H1                            N        0.000            ?│
  │UA005631601     H1                   0001J39S N        1.000            ?│
  │UA005810023     H1                   0001DTRR N        1.000            ?│
  │UA005810023     H1                   0001HZNE N        2.000            ?│
  │UA007514142     H1                   0001c3e9 N        1.000            ?│
  │UA007514142     H1                   0001c8v3 N        2.000            ?│
  │UA007621075     H1                   0001HA25 N        8.000            ?│
  │UA007621075     H1                   0001HD9L N       12.000            ?│
  └─────────────────────────────────────────────────────────────────────────┘

The distinction in the different values in the column "Turnarnd".

I've now idea what it can be related with...

Rerun the whole main set of tests.

When should I rerun testing: right now or at 22:00 - 4:00 of the Moscow time as Eugenie advised in the task #2787, note 28?

Follow Eugenie's advice.

For these particular testcases(gso_278 and tc_item_master_007) the start time is not so important, and you can start right now(at least before good time frame arriving you will have one more main part results).

Regression testing (main part) finished. Only one test failed: tc_job_002. So we can assume the testing is PASSED. I've rebased the branch. Now current revision is 11050. Can I commit the changes?

1. ThinClient - remove the org.apache.commons.collections import
2. AbstractGuiDriver has no functional changes (just a removed line, 2063)
3. SwingMouseHandler is missing history entry - but there are no functional changes, just formatting/import changes. Was this by intent? Or something left after rebase?
4. p2j.mouse.js has no functional changes

Vadim, after you address the issues in this note, as 2849a has gone through a few major rebase steps (with trunk revisions which affected UI), please run a some relevant tests for this task and a selection from the "Standalone GUI Test List" Greg posted in note 218. If 2849a is behaving properly with these tests, then you can merge.

Constantin Asofiei wrote:

Review for 2849a rev 11050

1. ThinClient - remove the org.apache.commons.collections import
2. AbstractGuiDriver has no functional changes (just a removed line, 2063)
3. SwingMouseHandler is missing history entry - but there are no functional changes, just formatting/import changes. Was this by intent? Or something left after rebase?

Probably some merging issue. There was a lot of manual merging during rebasing.

1. p2j.mouse.js has no functional changes

What to do with files where there are no functional changes?

Vadim, after you address the issues in this note, as 2849a has gone through a few major rebase steps (with trunk revisions which affected UI), please run a some relevant tests for this task and a selection from the "Standalone GUI Test List" Greg posted in note 218. If 2849a is behaving properly with these tests, then you can merge.

I did it recently. You want me to do it again?

Vadim Gindin wrote:

There was a lot of manual merging during rebasing.

Have you rechecked 2849a against trunk, to ensure you haven't lost any of your changes during rebase? If you think that something might have been missed, is better for you to do this, because you know all the changes which should or not be in 2849a.

For branches with complex changes, what I used to do was do a diff between the branch and its current trunk base (and save this diff into a file), rebase, do a diff again with current trunk, and diff (usually using meld) this two diff files... the two diff files should have the almost same content (there will be some garbage, like line numbers, but with some grepping you can eliminate those).

What to do with files where there are no functional changes?

Revert these files individually to the trunk revision (if the changes in the file do not relate to your task, i.e. there is only whitespace changes and the like).

I did it recently. You want me to do it again?

It depends on what trunk revision was 2849a based on when you did the tests. If it was before rebasing on trunk rev 11000 or rev 10996, is best to re-check, as these were very UI-related.

I've checked standalone tests, tested my tests again, fixed merging bug for browse, fixed label setting for horizontal/vertical scroll bars switching. Please review. These changes are small and mostly GUI specific. Should I run regression testing again? Actual rebased branch revision is 11056.

P.S. Constantin, thanks for advice. It will come in handy next time.

Vadim Gindin wrote:

Should I run regression testing again?

No, testing again is not needed, as all changes are in GUI.

If you are sure the 2849a merging is correct and nothing else was lost/altered during rebase, you can merge to trunk.

The branch is merged to trunk with bzr revision 11003.

I can close this task, right?

Also: please archive 2849a.

Greg Shah wrote:

I can close this task, right?

Yes, please.

Also: please archive 2849a.

Done.