Skip to content

Prevent Crash if Closing the DASD Context Menu without Action [SLE-15-SP5]#103

Merged
shundhammer merged 2 commits intoSLE-15-SP5from
huha-fix-context-menu
May 10, 2023
Merged

Prevent Crash if Closing the DASD Context Menu without Action [SLE-15-SP5]#103
shundhammer merged 2 commits intoSLE-15-SP5from
huha-fix-context-menu

Conversation

@shundhammer
Copy link
Copy Markdown
Contributor

@shundhammer shundhammer commented May 10, 2023

Target Branch / Release

This is for SLE-15-SP5. The merge to master is #104.

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

@shundhammer shundhammer force-pushed the huha-fix-context-menu branch from b71dbe9 to 3aa139e Compare May 10, 2023 14:00
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage: 21.262% (-0.02%) from 21.282% when pulling 3aa139e on huha-fix-context-menu into 68ae06f on SLE-15-SP5.

@shundhammer shundhammer requested a review from teclator May 10, 2023 14:10
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 d4f39d7 into SLE-15-SP5 May 10, 2023
@shundhammer shundhammer deleted the huha-fix-context-menu branch May 10, 2023 14:35
@yast-bot
Copy link
Copy Markdown
Contributor

✔️ Internal Jenkins job #757 successfully finished
✔️ Created IBS submit request #297459

@shundhammer shundhammer changed the title Prevent Crash if Closing the DASD Context Menu without Action Prevent Crash if Closing the DASD Context Menu without Action [SLE-15-SP5] May 10, 2023
@yast-bot
Copy link
Copy Markdown
Contributor

✔️ Internal Jenkins job #758 successfully finished
✔️ Created IBS submit request #297628

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