-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124659/
-----------------------------------------------------------

Review request for Plasma.


Repository: plasma-desktop


Description
-------

There are two scenarios that lead to segfault in kcm_keyboard.

1. Run kcmshell5 keyboard; switch to the second tab; in layouts table 
double-click on item in third column so that combo box appears; click on empty 
space in layouts table; [Preview] button does not get deactivated; click on 
that button and see kcm_keyboard crashing.
2. Run kcmshell5 keyboard; switch to the second tab; modify something so that 
[Reset] button becomes active; select any row in layouts table; click on 
[Reset]; row becomes deselected, but [Preview] button is still active; click on 
that button and see kcm_keyboard crashing.

[Preview] button should become inactive when no rows are selected, but in these 
two scenarios it doesn't. KCMKeyboardWidget::layoutSelectionChanged slot does 
not get called in these two cases. What happens in described cases:

1. layoutsTableModel emits dataChanged signal. KCMKeyboardWidget::uiChanged 
slot gets called. LayoutsTableModel::refresh is called, then in 
QAbstractItemModel::endResetModel modelReset signal is emitted, 
QAbstractItemView::reset slot gets called, and it calls 
QItemSelectionModel::reset. QItemSelectionModel::reset disables signals and 
calls QItemSelectionModel::clear that calls 
QItemSelectionModel::clearSelection, but signals are disabled, so 
itemSelectionChanged is not emitted, and 
KCMKeyboardWidget::layoutSelectionChanged is not called.
2. KCMKeyboard::load calls KCMKeyboardWidget::updateUI that calls 
LayoutsTableModel::refresh. All following calls are in the same order as in 
case 1.

I propose to call KCMKeyboardWidget::layoutSelectionChanged manually after 
problematic refreshes to update buttons state. It's not the best fix, there 
still may be places where manual call of layoutSelectionChanged is needed, but 
at least it fixes two segfaults.


Diffs
-----

  kcms/keyboard/kcm_keyboard_widget.cpp 78ec60b 

Diff: https://git.reviewboard.kde.org/r/124659/diff/


Testing
-------


Thanks,

Maxim Mikityanskiy

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to