yast / yast-yast2

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

Force Creating the UI before Checking -pkg etc. UI Plug-ins #1204

Closed shundhammer closed 2 years ago

shundhammer commented 2 years ago

Bugzilla

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

Trello

https://trello.com/c/bM1EFCcS

Problem

When clicking on a downloaded RPM in the desktop or in the web browser, there was a crash.

Cause

This uses a MIME association between MIME type application/x-rpm and a YaST .desktop file that ultimately calls y2start sw_single <mypackage.rpm>.

But because this adds arguments to the command line, it does not create a UI immediately (i.e. it does not load and execute a libyui plug-in); it delays that as long as possible because normally, YaST in CLI (command line interface) mode is not supposed to open any windows if it can be avoided at all.

In this case, however, it always opens a window anyway: That's the whole point of using the sw_single YaST module instead of a zypper command line like zypper in <mypackage.rpm>: It shows progress bars, and if any dependency problems are reported, the user can solve them with the Qt or NCurses UI.

Still, the new part that checks if the associated -pkg UI extension (libyui-qt-pkg or libyui-ncurses-pkg) is installed (and asks if it should install it if it's not) failed because it didn't even find the main UI plug-in.

Fix

This commit executes a simple UI operation that does nothing, but as a side effect it initiates loading the UI, so the UI extension checker can do its job.

Suggested Alternative

@jreidinger suggested to add another exception to the code in the YaST ruby bindings that lists those YaST modules that should, unlike all others, not inhibit creating the UI if command line arguments are detected:

https://github.com/yast/yast-ruby-bindings/blob/master/src/y2start/y2start#L55

IMHO this is clumsy, fragile and dangerous, so I decided to explicitly force the UI to be created when that checker needs it.

Already we have a number of YaST modules where that UIExtensionChecker is used:

...which of course means that we would have to list every single one of them in that yast-ruby-bindings code, and if we get more of them, we would have to remember to list them there as well. That does not sound like a stable and future-proof solution to me.

Flexibility

The UIExtensionChecker now has another optional parameter force_ui (default: true) in its constructor. If enforcing creating the UI is not desired, the caller can pass false for that parameter; but in that case, the checks need to be moved to the caller level.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.006%) to 41.013% when pulling 62ae9394e70eb0e53bbd962ade157bfbc2c78119 on huha-force-ui into 58a49bb97c99df98a90a169399f36c5aeb33af39 on master.

jreidinger commented 2 years ago

fix looks ok for me. Just can you attach screenshot of opened window? As code in ruby-bindings also ensure proper application window title, so to check if it is correct.

shundhammer commented 2 years ago

sw_single

yast-bot commented 2 years ago

:heavy_check_mark: Public Jenkins job #330 successfully finished :heavy_check_mark: Created OBS submit request #931637

yast-bot commented 2 years ago

:heavy_check_mark: Internal Jenkins job #170 successfully finished :heavy_check_mark: Created IBS submit request #258546