yast / yast-yast2

YaST module yast2
http://en.opensuse.org/Portal:YaST
GNU General Public License v2.0
55 stars 44 forks source link

Prevent Crash if Replacing Content of Nonexistent Replace Point [merge to master] #1182

Closed shundhammer closed 3 years ago

shundhammer commented 3 years ago

This merges PR #1181 from SLE-15-SP3 to master.

Trello

https://trello.com/c/wC5C2kRW

Bugzilla

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

Problem

Under some circumstances, updating progress in the Progress module can lead to a crash with a UI error "No widget with ID ...".

Probable Cause

Another pop-up dialog may be in the way, e.g. during some libzypp callbacks. That will then be the topmost dialog, and of course that dialog does not have the expected widget tree, i.e. not the ReplacePoint that the Progress module wants to modify with UI.ReplaceWidget().

Fix

Be more defensive in that Progress module: Check if the expected widget exists in the current dialog before trying to exchange its content.

Test

There was no test environment for the Progress module at all: The only way of testing it was with a live YaST module. But that makes it complicated to get into all cases; and, worse for this bug, there is no way to enforce a popup dialog being in the way.

So I added a new manual test client for this. It's not realistically possible to do that with auto-tests, so this needs some manual interaction and watching what happens.

The new test client is started from the test directory:

cd yast-yast2/library/wizard/test/manual
yast2 ./progress_demo

or, for NCurses:

yast2 ./progress_demo --ncurses

It first asks which progress scenario to use:

progress-demo-01-ask-type

Then it opens one of either:

progress-demo-02-simple-main Simple progress

progress-demo-04-complex-main Complex progress

In either case, there are some buttons which directly correspond to the exported actions of the Progress module, and you can open one or more pop-up dialogs that get in the way (which is the purpose of this test for this bug). That pop-up has the same buttons to test the Progress module's behaviour.

progress-demo-03-simple-popup With popup dialog in the way

Clicking around on those buttons might not update the progress output correctly, but it should at least not crash with a UI exception and error dialog.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.009%) to 40.974% when pulling 1bf002645b668f35df80a2d07023859686714ce3 on huha-merge-15-sp3-to-master into 6565a5395d7e56d387bbfc977113dff0fe420e74 on master.

yast-bot commented 3 years ago

:heavy_check_mark: Internal Jenkins job #155 successfully finished :heavy_check_mark: Created IBS submit request #245171

yast-bot commented 3 years ago

:heavy_check_mark: Public Jenkins job #317 successfully finished :heavy_check_mark: Created OBS submit request #906456