yast / yast-yast2

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

Prevent Crash if Replacing Content of Nonexistent Replace Point #1181

Closed shundhammer closed 3 years ago

shundhammer commented 3 years ago

This is the fix for SLE-15-SP3. The merge to master will follow.

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 decreased (-0.01%) to 40.53% when pulling 255b9c6ceb2210c53ba33dd429acc6c1f39b6732 on huha-replace-widget into 8bc144d90953dac45686dc3fad8bb5b961234d3c on SLE-15-SP3.

shundhammer commented 2 years ago

Notice that even though this maintenance request was declined, it was superseded by https://build.suse.de/request/show/245245 which was accepted and also includes these changes.