Project

General

Profile

Bug #10693

Incorrect z-order and focus for new Stack trace dialog

Added by Sergey Ivanovskiy 10 months ago. Updated 8 months ago.

Status:
Merge Pending
Priority:
Normal
Assignee:
Eugenie Lyzenko
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
trunk 16301
reviewer:
Hynek Cihlar
production:
No
env_name:
topics:

Description

I see the modal dialog windows are incorrectly placed one upon another. It can be observed for Hotel CHUI web client. Go to Room management and Press A to activate the current room. The Activate room modal dialog appears with Yes, No and Help buttons. Press Help button that opens the Stack trace modal dialog with OK and Cancel buttons that becomes displayed under its parent dialog and this dialog does not have a focus. The focus persists with its parent dialog (the Activate room modal dialog).

stack_trace_fixed.jpg - Fixed stack trace ChUI (58.5 KB) Eugenie Lyzenko, 11/26/2025 03:43 PM

History

#1 Updated by Sergey Ivanovskiy 8 months ago

The modal dialog windows are incorrectly placed one upon another. It can be observed for Hotel CHUI web client. Go to Room management and Press A to activate the current room. The Activate room modal dialog appears with Yes, No and Help buttons. Press Help button that opens the Stack trace modal dialog with OK and Cancel buttons that becomes displayed under its parent dialog and this dialog does not have a focus. The focus persists with its parent dialog (the Activate room modal dialog).

#2 Updated by Sergey Ivanovskiy 8 months ago

Please see #10359-47.

#3 Updated by Eugenie Lyzenko 8 months ago

Sergey Ivanovskiy wrote:

Please see #10359-47.

This is not related to GUI modal/always-on-top issue. It is completely different case specific to ChUI mode implementation details. So the fix can take a time. I do not this it could be ready on Monday, sorry.

#4 Updated by Eugenie Lyzenko 8 months ago

  • Status changed from New to WIP

There are several issues in this task discovered during debug. What I can see now is:

1. The StackTrace frame/window has very wrong lay out. It should be window/frame with border and title including editor widget which should be also bordered and two buttons to accept the user input. Nothing of this work properly.

2. The main issue is the stack trace frame is below the alert box and does not react to the user input. And this is expected and never worked properly. The reason is the alert box is modal window running inside it's own modal event loop. So to make the stack trace over this alert box we need to create new modal window and introduce yet another modal event loop.

So far I'm going to prepare new special class for ChUI, say StackTraceChuiWindow to implement IsModal interface and introduce new modal loop. I think it should work. Continue working.

#5 Updated by Eugenie Lyzenko 8 months ago

Created task branch 10693a from trunk revision 16293.

#6 Updated by Eugenie Lyzenko 8 months ago

The branch 10693a has been updated for review to revision 16294.

This is the fix for original issue. The stack trace looks now:

Fixed stack trace ChUI

For z-order issue we do not need the new modal event loop. We can live inside one from alert box. But several hacks are necessary. Also look and feel resolved.

#7 Updated by Hynek Cihlar 8 months ago

  • reviewer Hynek Cihlar added

#8 Updated by Eugenie Lyzenko 8 months ago

The branch 10693a has been rebased with trunk 16295, new version is 16296.

#9 Updated by Hynek Cihlar 8 months ago

Code review 10693a.

The changes look fine. Just a question, was the check isStackTraceActive in AlertBoxImpl added to address the z-order issue? Please put a comment there to explain its purpose.

#10 Updated by Eugenie Lyzenko 8 months ago

Hynek Cihlar wrote:

Code review 10693a.

The changes look fine. Just a question, was the check isStackTraceActive in AlertBoxImpl added to address the z-order issue? Please put a comment there to explain its purpose.

The branch 10693a has been updated for review to revision 16297.

Added comments to clarify isStackTraceActive() usage details.

#11 Updated by Eugenie Lyzenko 8 months ago

Are there any more notes/objections. I would like to merge this into trunk.

#12 Updated by Eugenie Lyzenko 8 months ago

The branch 10693a has been rebased with trunk 16298, new version is 16300.

#13 Updated by Hynek Cihlar 8 months ago

  • Status changed from Review to Internal Test

This will require ChUI regression testing.

#14 Updated by Eugenie Lyzenko 8 months ago

Hynek Cihlar wrote:

This will require ChUI regression testing.

We can do ChUI harness for this. But it is virtually useless. Because the harness does not have the tests to fire the bug(otherwise we already saw the issue). And for regular functionality nothing has changed.

#15 Updated by Hynek Cihlar 8 months ago

Eugenie Lyzenko wrote:

Hynek Cihlar wrote:

This will require ChUI regression testing.

We can do ChUI harness for this. But it is virtually useless. Because the harness does not have the tests to fire the bug(otherwise we already saw the issue). And for regular functionality nothing has changed.

The purpose is to spot any potential regressions, not to functionally test the implemented changes.

#16 Updated by Eugenie Lyzenko 8 months ago

Hynek Cihlar wrote:

Eugenie Lyzenko wrote:

Hynek Cihlar wrote:

This will require ChUI regression testing.

We can do ChUI harness for this. But it is virtually useless. Because the harness does not have the tests to fire the bug(otherwise we already saw the issue). And for regular functionality nothing has changed.

The purpose is to spot any potential regressions, not to functionally test the implemented changes.

Yes, I do completely agree. But changed code showing stack trace is not executed in ChUI harness. So even if there are regressions with this branch we will not see it in testcases.

But I can start testing anyway. I just do not see the positive effect from this action.

#17 Updated by Hynek Cihlar 8 months ago

Eugenie Lyzenko wrote:

Hynek Cihlar wrote:

Eugenie Lyzenko wrote:

Hynek Cihlar wrote:

This will require ChUI regression testing.

We can do ChUI harness for this. But it is virtually useless. Because the harness does not have the tests to fire the bug(otherwise we already saw the issue). And for regular functionality nothing has changed.

The purpose is to spot any potential regressions, not to functionally test the implemented changes.

Yes, I do completely agree. But changed code showing stack trace is not executed in ChUI harness. So even if there are regressions with this branch we will not see it in testcases.

But I can start testing anyway. I just do not see the positive effect from this action.

You are right, in this case it's likely not needed. I generally always run ChUI regression tests (or any regression tests) when I touch ChUI code even when I'm pretty sure there are zero side effects. It costs nothing. And it prevents surprises.

I wasn't sure there were no stack traces shown (from any recent changes for example, or plugins) that is why I suggested the tests.

#18 Updated by Eugenie Lyzenko 8 months ago

Hynek Cihlar wrote:

Eugenie Lyzenko wrote:

Hynek Cihlar wrote:

Eugenie Lyzenko wrote:

Hynek Cihlar wrote:

This will require ChUI regression testing.

We can do ChUI harness for this. But it is virtually useless. Because the harness does not have the tests to fire the bug(otherwise we already saw the issue). And for regular functionality nothing has changed.

The purpose is to spot any potential regressions, not to functionally test the implemented changes.

Yes, I do completely agree. But changed code showing stack trace is not executed in ChUI harness. So even if there are regressions with this branch we will not see it in testcases.

But I can start testing anyway. I just do not see the positive effect from this action.

You are right, in this case it's likely not needed. I generally always run ChUI regression tests (or any regression tests) when I touch ChUI code even when I'm pretty sure there are zero side effects. It costs nothing. And it prevents surprises.

I wasn't sure there were no stack traces shown (from any recent changes for example, or plugins) that is why I suggested the tests.

OK. Let's do this testing.

#19 Updated by Eugenie Lyzenko 8 months ago

The ChUI testing passed without new issues. I see out of memory in harness test but this can be related to moving to Java 17 and increased memory consumption.

#20 Updated by Hynek Cihlar 8 months ago

  • Status changed from Internal Test to Merge Pending

Please merge 10693a after 10085a.

#21 Updated by Eugenie Lyzenko 8 months ago

Hynek Cihlar wrote:

Please merge 10693a after 10085a.

OK. Starting merge now.

#22 Updated by Eugenie Lyzenko 8 months ago

  • version_resolved set to trunk 16301

Branch 10693a was merged to trunk as revno 16301 then it was archived.

Also available in: Atom PDF