Project

General

Profile

Bug #2850

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

menu/simple_sm.p drawing issues in Swing client

Added by Greg Shah over 5 years ago. Updated over 4 years ago.

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

100%

billable:
No
vendor_id:
GCD
case_num:

menubar_background.png (1.67 KB) Vadim Gindin, 11/19/2015 10:03 AM

1123.diff Magnifier (14.1 KB) Vadim Gindin, 11/23/2015 03:09 AM

1124.diff Magnifier (9.14 KB) Vadim Gindin, 11/24/2015 12:18 PM

2850_2511.diff Magnifier (4.96 KB) Vadim Gindin, 11/25/2015 11:52 AM

2711_rev10976.diff Magnifier (4.52 KB) Vadim Gindin, 11/27/2015 12:04 PM

History

#1 Updated by Greg Shah over 5 years ago

Using ./menu/simple_sm.p in the Swing client, Vadim reported:

1. After I press on some menu-item in the on of sub-menus of a menubar, root sub-menu in menubar stays "mouse-entered" after body is hidden.

2. The same scenario: sub-menu child is still displayed as focused after it's parent's body is hidden.

3. At the start of the test the warning "Run-time error ... (3269)" is shown and no menubar is displayed in PROGRESS until the user will press any key, but in P2J empty menubar rectangle is drawn.

#2 Updated by Vadim Gindin over 5 years ago

  • File fonts_compare.png added
  • File hidden_menu_elems.png added
Here are the images
  1. Fonts compare of PROGRESS, P2J SWING, P2J WEB. There is an other font for WEB.
  2. MENU bug of showing hidden menu items

#3 Updated by Vadim Gindin over 5 years ago

  • File deleted (hidden_menu_elems.png)

#4 Updated by Vadim Gindin over 5 years ago

  • File deleted (fonts_compare.png)

#5 Updated by Vadim Gindin over 5 years ago

Greg Shah wrote:

3. At the start of the test the warning "Run-time error ... (3269)" is shown and no menubar is displayed in PROGRESS until the user will press any key, but in P2J empty menubar rectangle is drawn.

The place where that background is drawn is the same as for popup_ext.p: BorderedPanelGuiImpl.draw(). I've checked that at the drawing time menubar is hidden and it does not post PaintEvent's. So there are some other widget posted clipping rectangle, that intersects menubar area.. Once again if the widget is responsible to draw it's background itself, why it is needed to draw some rectangles in BorderedPanelGuiImpl.draw()?

#6 Updated by Vadim Gindin over 5 years ago

I'm trying to understand how the "menubar's" rectangle get into screen clippings, that are drawn in BorderedPanelGuiImpl.draw(). I recall that at that moment MENUBAR itself is hidden and does not post PaintEvent, so there is some other widget, that posts a PaintEvent with similar rectangle. I.e. right after start, when the warning "Run-time error in WAIT-FOR at line ..." is shown an a messagebar menubar is still hidden but it's background rectangle is drawn:

I've added logging of processed PaintEvent native rectangle and source to TitledWindow.processEvent() method (line 163). Here is what I've got (top, left, bottom, right):

PE: [0, 0, 41, 2147483647], source=MessageAreaGuiImpl
PE: [0, 0, 29, 399], source=MessageAreaGuiImpl
PE: [0, 0, 41, 399], source=BorderedPanelGuiImpl
PE: [0, 0, 29, 399], source=BorderedPanelGuiImpl
PE: [0, 0, 2147483647, 2147483647], source=WindowTitleBar
PE: [0, 0, 2147483647, 399], source=WindowTitleBar
PE: [0, 0, 2147483645, 2147483645], source=WindowTitleBar$WindowIcon
PE: [1, 2, 2147483646, 2147483647], source=WindowTitleBar$WindowIcon
PE: [0, 0, 2147483647, 399], source=WindowTitleBar$WindowTitle
PE: [0, 0, 17, 399], source=WindowTitleBar$WindowTitle
PE: [0, 0, 18, 399], source=WindowTitleBar
PE: [0, 0, 13, 15], source=CaptionButton
PE: [2, 382, 15, 397], source=CaptionButton
PE: [0, 0, 13, 15], source=CaptionButton
PE: [2, 364, 15, 379], source=CaptionButton
PE: [0, 0, 13, 15], source=CaptionButton
PE: [2, 348, 15, 363], source=CaptionButton
PE: [1, 19, 18, 399], source=WindowTitleBar$WindowTitle
PE: [1, 19, 18, 345], source=WindowTitleBar$WindowTitle
PE: [0, 0, 541, 408], source=WindowGuiImpl
PE: [23, 4, 484, 403], source=WindowWorkSpace
PE: [23, 4, 484, 403], source=BorderedPanelGuiImpl
PE: [0, 0, 540, 407], source=BorderedPanelGuiImpl
PE: [4, 4, 22, 403], source=WindowTitleBar
PE: [485, 4, 514, 403], source=MessageAreaGuiImpl
PE: [515, 4, 536, 403], source=StatusLineGuiImpl
PE: [485, 4, 514, 403], source=MessageAreaGuiImpl
PE: [485, 4, 514, 403], source=BorderedPanelGuiImpl
PE: [4, 4, 22, 403], source=WindowTitleBar
PE: [5, 6, 20, 21], source=WindowTitleBar$WindowIcon
PE: [5, 23, 22, 349], source=WindowTitleBar$WindowTitle
PE: [6, 386, 19, 401], source=CaptionButton
PE: [6, 368, 19, 383], source=CaptionButton
PE: [6, 352, 19, 367], source=CaptionButton
PE: [23, 4, 484, 403], source=WindowWorkSpace
PE: [23, 4, 484, 403], source=BorderedPanelGuiImpl
PE: [0, 0, 541, 408], source=WindowGuiImpl
PE: [0, 0, 564, 408], source=WindowGuiImpl
PE: [0, 0, 540, 407], source=BorderedPanelGuiImpl
PE: [0, 0, 563, 407], source=BorderedPanelGuiImpl
PE: [508, 4, 537, 403], source=MessageAreaGuiImpl
PE: [538, 4, 559, 403], source=StatusLineGuiImpl
PE: [508, 4, 537, 403], source=MessageAreaGuiImpl
PE: [538, 4, 559, 403], source=StatusLineGuiImpl
PE: [4, 4, 22, 403], source=WindowTitleBar
PE: [0, 0, 564, 408], source=WindowGuiImpl
PE: [4, 4, 22, 403], source=WindowTitleBar
PE: [4, 4, 22, 403], source=WindowTitleBar
PE: [0, 0, 564, 408], source=WindowGuiImpl
PE: [4, 4, 22, 403], source=WindowTitleBar
PE: [4, 4, 22, 403], source=WindowTitleBar

Menubar rectangle starts at top=23px approximately, so I don't see here exact rectangle, but here are some intersecting rectangles and some rectangles seems strange:
PE: [0, 0, 41, 2147483647], source=MessageAreaGuiImpl
PE: [0, 0, 29, 399], source=MessageAreaGuiImpl
PE: [0, 0, 41, 399], source=BorderedPanelGuiImpl
PE: [0, 0, 29, 399], source=BorderedPanelGuiImpl

Message area resides in the bottom of the window. It looks that rectangles are incorrect. What do you think?

#7 Updated by Constantin Asofiei over 5 years ago

Vadim, in which branch are you working this?

Also, please remind me, have you managed to address the sub-menu body repaint I mentioned some time ago?

#8 Updated by Constantin Asofiei over 5 years ago

Vadim, I think I know why you are seeing the top light-gray space. Menu-bar gets attached to window (and window is relayout) BEFORE making it visible in GUI. At the time this code is called and warning is shown:

         if(!hasEnabledFrame())
         {
            warnNoEnabledFrames(wnd);
         }

         wnd.getMenubar().setVisible(true);

the window is already layout as if a menu-bar is added. Only later the menu-bar is being drawn (as it is being made visible).

Can you check what happens if the menubar gets set to not-visible via a trigger? Is the window workspace resized and the menubar hidden? If so, the window needs to layout and use the menubar only if the menubar is visible; and if the menubar's visibility gets changed, the window needs to be relayout.

#9 Updated by Vadim Gindin over 5 years ago

Constantin Asofiei wrote:

Vadim, in which branch are you working this?

bracnh 2677a, revno 11067.

Also, please remind me, have you managed to address the sub-menu body repaint I mentioned some time ago?

You probably mean the note 22 in the task #2736. Yes, I'd tried that, but unsuccessfully: sub-menu's aren't drawn. You can see details about it in that task #2736 the latest post (popup_ext.p). More over I think there is another reason in that task.

#10 Updated by Vadim Gindin over 5 years ago

Constantin. You were absolutely right about the reason. I've added isVisible check for MENUBAR to WindowLayout and fixed dynamic attach/detach case for GUI and CHUI. By the way, MENUBAR can be shown only by assigning to window:menubar attribute. visible attribute is not setable for menu. I've added corresponding error. I've also fixed CHUI layout for dynamic case.

Attached diff contains my current changes.

#11 Updated by Greg Shah over 5 years ago

Code Review 1123.diff

1. I don't think it is a good idea to require that the status line and message line implementations must calculate whether or not the menubar exists to determine their location. The containing window should have a method that reports the correct location for each one. Actually, I don't understand why the location changes when the menubar is present. The screen dimensions should not be different and the status/message lines are at the same location no matter if the menubar exists or not.

2. You have mixed in your other changes (right mouse issues and toggle-box issues). I don't think the right mouse changes are safe, so these cannot be included.

Constantin: please review.

#12 Updated by Constantin Asofiei over 5 years ago

Beside what Greg noted:
  • in ChUI, message area, status area, frames (and menubar) are all attached to the same parent (in other terms, they are all siblings). So the change in WindowChuiImpl doesn't look correct, as the contentPane will contain everything, not just frames
  • in ChUI, we don't have (yet) a container separation between frames and message area/status area/menubar. This requires more testing, to determine if a frame can overlay the last 3 rows (message + status) in the screen. If it can not, then ChUI needs a "workspace" equivalent, as GUI has, to separate the frames.
  • as Greg mentioned, the location/height of the status/message area is constant. Only the height available for frame placement is reduced when the menubar is attached in ChUI.

#13 Updated by Vadim Gindin over 5 years ago

So should I fix it now or postpone that?

#14 Updated by Greg Shah over 5 years ago

1. Separate out the changes for toggle-box/right mouse issues.

2. Fix the rest based on the code reviews.

#15 Updated by Vadim Gindin over 5 years ago

All 3 points in this task are fixed. See attached diff. I've separated issues from other tasks.

#16 Updated by Greg Shah over 5 years ago

  • Assignee set to Vadim Gindin

Code Review 1124.diff

My only question is regarding the addition of the Window.currentFocus() code. This seems wrong. If I am understanding it properly, as long as there is ever a popup menu in the widget list for a window, only that popup menu will ever be focusable.

Please explain.

#17 Updated by Vadim Gindin over 5 years ago

Greg Shah wrote:

Code Review 1124.diff

My only question is regarding the addition of the Window.currentFocus() code. This seems wrong. If I am understanding it properly, as long as there is ever a popup menu in the widget list for a window, only that popup menu will ever be focusable.

Please explain.

It is just a such behaviour: while popup menu is opened, tab order isn't working and navigation is available only inside menu using arrows. It probably relates to #2849 task for ignoring right mouse button: when we do not set a focus to the button, on which we clicked using right mouse button, we will not be able to find current focused item in menu.. I can also move it at this point from the current update to #2849 changes set.

P.S.
Another bug for CHUI: Focused element of MENUBAR must be drawn as Color.MESSAGES, but it is drawn as Color.NORMAL. I recall that Color.MESSAGES=Color.NORMAL.withReverse().

#18 Updated by Greg Shah over 5 years ago

It is just a such behaviour: while popup menu is opened, tab order isn't working and navigation is available only inside menu using arrows. It probably relates to #2849 task for ignoring right mouse button: when we do not set a focus to the button, on which we clicked using right mouse button, we will not be able to find current focused item in menu.

I have 2 concerns:

1. When the popup menu is not open, the currentFocus() will be broken.

2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.

#19 Updated by Vadim Gindin over 5 years ago

Greg Shah wrote:

It is just a such behaviour: while popup menu is opened, tab order isn't working and navigation is available only inside menu using arrows. It probably relates to #2849 task for ignoring right mouse button: when we do not set a focus to the button, on which we clicked using right mouse button, we will not be able to find current focused item in menu.

I have 2 concerns:

1. When the popup menu is not open, the currentFocus() will be broken.

In that case w.isVisible()==false and therefore super.currentFocus() will be called. Isn't it?

2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.

The difference is in the fact, than Menu holds it's own widgets tree of menu widgets, that are also focusable.

#20 Updated by Greg Shah over 5 years ago

In that case w.isVisible()==false and therefore super.currentFocus() will be called. Isn't it?

Yes, you're probably right.

2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.

The difference is in the fact, than Menu holds it's own widgets tree of menu widgets, that are also focusable.

The key point here is that we don't want to spread widget-specific behavior (e.g. for menus) into other non-related classes (e.g. window). It makes it hard to understand how menus work. It makes it hard to find all the behavior related to menus. It makes those classes very dependent upon one another, which makes things fragile and more likely to break.

Constantin/Hynek: thoughts?

#21 Updated by Hynek Cihlar over 5 years ago

Greg Shah wrote:

In that case w.isVisible()==false and therefore super.currentFocus() will be called. Isn't it?

Yes, you're probably right.

2. I don't think that the popup menu should be treated as a widget contained by the window. It is more like the combo-box drop-down. It temporarily appears and when it is open it owns the interaction with the user.

The difference is in the fact, than Menu holds it's own widgets tree of menu widgets, that are also focusable.

The key point here is that we don't want to spread widget-specific behavior (e.g. for menus) into other non-related classes (e.g. window). It makes it hard to understand how menus work. It makes it hard to find all the behavior related to menus. It makes those classes very dependent upon one another, which makes things fragile and more likely to break.

Constantin/Hynek: thoughts?

I think Greg is right. Menu should be rendered in its own top-level window, otherwise we will have the same issue as with combo drop-down - the menu won't be able to draw outside of its logical parent window.

#22 Updated by Vadim Gindin over 5 years ago

OK, I understand.

I have collected changes for 2-3 tasks and I would want to commit some of them, that are approved. All issues in this task are approved. Can I commit them (the latest diff) excluding Menu change?

Talking about ability to draw menu with opened sub-menus exceeding window sizes, is there a separate task for this already? Is it solved for combo-box or other widget, that has the same issue?

#23 Updated by Hynek Cihlar over 5 years ago

Vadim Gindin wrote:

Talking about ability to draw menu with opened sub-menus exceeding window sizes, is there a separate task for this already? Is it solved for combo-box or other widget, that has the same issue?

There is an issue for combo-box drop-down, see #2837.

#24 Updated by Vadim Gindin over 5 years ago

Here is the latest diff on merged branch.

#25 Updated by Vadim Gindin over 5 years ago

Yesterday, I've fixed CHUI bug with menubar:

Another bug for CHUI: Focused element of MENUBAR must be drawn as Color.MESSAGES, but it is drawn as Color.NORMAL. I recall that Color.MESSAGES=Color.NORMAL.withReverse().

So once again can I commit the last change (diff in the note #24)?

#26 Updated by Vadim Gindin over 5 years ago

Here is the current changes diff (base on revision 10976 of the branch 2677b). Changes solve 3 bugs, described in the note 1 of this task. Please review and approve commit.

#27 Updated by Greg Shah over 5 years ago

Go ahead and check these into 2677b.

#28 Updated by Vadim Gindin over 5 years ago

Committed with revno 10978.

#29 Updated by Greg Shah over 5 years ago

Are there any issues mentioned in this task which are not yet fixed AND which are not part of some other open task?

#30 Updated by Vadim Gindin over 5 years ago

There are the following issues:
  1. CHUI. Dynamic attach/detach menubar (MENUBAR layout problems). It correlates with the task #2557
  2. CHUI. Fix pop-up menu layout (popup-ext.p).
  3. GUI. The case when opened sub-menus can reside outside of the window. It correlates with the task #2837.

#31 Updated by Greg Shah over 5 years ago

CHUI. Dynamic attach/detach menubar (MENUBAR layout problems). It correlates with the task #2557

It was my understanding that this was a GUI-only issue as noted in #2557.

CHUI. Fix pop-up menu layout (popup-ext.p).

This is still broken? Is it already being worked in another task?

If it is broken and not being worked, please create a new task for this and post the task number here.

I plan to close this task once that task is created, unless you know of a reason I should not.

#32 Updated by Vadim Gindin over 5 years ago

I've created the task #2933 for CHUI layout errors.

#33 Updated by Greg Shah over 5 years ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed

#34 Updated by Greg Shah over 4 years ago

  • Target version changed from Milestone 12 to GUI Support for a Complex ADM2 App

Also available in: Atom PDF