Skip to content

Prevent Crash if Closing the DASD Context Menu without Action [master]#104

Merged
shundhammer merged 4 commits intomasterfrom
huha-fix-context-menu-master
May 10, 2023
Merged

Prevent Crash if Closing the DASD Context Menu without Action [master]#104
shundhammer merged 4 commits intomasterfrom
huha-fix-context-menu-master

Conversation

@shundhammer
Copy link
Copy Markdown
Contributor

Target Branch / Release

This is the merge to master / Factory / TW of #103.

Bugzilla

https://bugzilla.suse.com/show_bug.cgi?id=1211213

Problem

Closing the context menu of the DASD list without executing any of its actions (e.g. by clicking outside the context menu or with the [Esc] key) caused a hard crash:

uninitialized constant Y2S390::DasdActions::Cancel

Cause

The UI.YContextMenu used here sends a CancelEvent (ID :cancel) when the user closes the menu without action. The yast-s390 code simply converted the received ID into a Ruby symbol for an action class to be executed and tried to run that class.

Fix

  • Now explicitly catching action :cancel and ignoring it.

  • Now generally checking if there actually is an action class for that symbol and logging an error instead of just crashing:

2023-05-10 15:20:39 <3> balrog(19572) [Ruby] dasd/dialogs.rb(rescue in run):150 No action for cancel: uninitialized constant Y2S390::DasdActions::Cancel

Test

Manual test with mocked data as described here:

  • Selecting a DASD from the list and opening the context menu
  • Clicking outside the context menu to close it
  • Opening it again and using the Esc key to close it

No complaint should appear in the log for this :cancel action.

  • Commented out the new return false if action == :cancel line and trying again: Now there should be an error in the y2log No action for cancel: uninitialized constant Y2S390::DasdActions::Cancel.

Other affected Places in YaST

This one place in the yast2-s390 DASD handling is really the only place (that we found) all over YaST where we use that YContextMenu, so the problem should not appear anywhere else.

Fix in the UI

The behavior of that YContextMenu is very weird. Closing a menu without any action should never send any event (i.e. make UI.UserInput() return any value); it should just silently close the menu, and that should be it.

But that behavior has been unchanged since 15 years ago, so there is the slight chance that other Open Source software which uses libyui may be affected. But sending a CancelEvent in that case is almost certainly always unwelcome, so fixing it also in our own upstream libyui-qt should only be an improvement.

Related PR

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage: 21.251% (-0.02%) from 21.267% when pulling 12d16c7 on huha-fix-context-menu-master into 91e158c on master.

Copy link
Copy Markdown
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shundhammer shundhammer merged commit 6f5e670 into master May 10, 2023
@shundhammer shundhammer deleted the huha-fix-context-menu-master branch May 10, 2023 15:09
@yast-bot
Copy link
Copy Markdown
Contributor

✔️ Public Jenkins job #41 successfully finished
✔️ Created OBS submit request #1086103

@shundhammer shundhammer changed the title Prevent Crash if Closing the DASD Context Menu without Action [SLE-15-SP5] Prevent Crash if Closing the DASD Context Menu without Action [master] May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants