All items had an index of -1. In previous versions, items had implicitly gotten an index corresponding to their position in the array. But with nested items, that's no longer so easy because child items also need to get an index.
That index is a somewhat ambiguous thing in the NCurses UI: Sometimes it really means the position of an item in the items array; sometimes it's just an arbitrary number that serves to identify the item (and find it; possibly after items were sorted, so their position in the array has become meaningless for identifying).
From the libyui perspective, making any assumption on YItem::index() is wrong; but for historical reasons, libyui-ncurses still does it in many places.
Fix
This PR ensures that NCTablePadBase::Append( NCTableCol ) does not silently fall back to using an index -1 anymore; that had been used for NCMultiSelectionBox and also for NCSelectionBox.
Now there is no longer a default value for the index parameter of that method, and both code locations using that function now explicitly pass the position in the array. Since it is used only in the addItem() method that is reimplemented from the YSelectionWidget parent class, that is as easy as using the previous YSelectionWidget::itemCount() just before calling the YSelectionWidget::addItem() parent class method.
Removing the default parameter ensures that this can now no longer be used by accident. All similar other NCTablePadBase::Append() variants do not have that problem since they use an NCTableLine that was created by the caller, and NCTableLine has its own _index member variable that is set (typically from the corresponding YItem) in its constructor.
Test
Use this new UI example and change the "Vegetable" check box. Watch the output fields and the y2log.
Trello
https://trello.com/c/ZbkIbc0M/2133-3-probable-libyui-regressions-p1-1177985-and-p1-1177982
Bugzilla
Symptom
In the NCurses UI,
UI.ChangeWidget(..., SelectedItems)kept failing with this exception:Problem
All items had an index of -1. In previous versions, items had implicitly gotten an index corresponding to their position in the array. But with nested items, that's no longer so easy because child items also need to get an index.
That index is a somewhat ambiguous thing in the NCurses UI: Sometimes it really means the position of an item in the items array; sometimes it's just an arbitrary number that serves to identify the item (and find it; possibly after items were sorted, so their position in the array has become meaningless for identifying).
From the libyui perspective, making any assumption on
YItem::index()is wrong; but for historical reasons, libyui-ncurses still does it in many places.Fix
This PR ensures that
NCTablePadBase::Append( NCTableCol )does not silently fall back to using an index -1 anymore; that had been used for NCMultiSelectionBox and also for NCSelectionBox.Now there is no longer a default value for the
indexparameter of that method, and both code locations using that function now explicitly pass the position in the array. Since it is used only in theaddItem()method that is reimplemented from the YSelectionWidget parent class, that is as easy as using the previousYSelectionWidget::itemCount()just before calling theYSelectionWidget::addItem()parent class method.Removing the default parameter ensures that this can now no longer be used by accident. All similar other
NCTablePadBase::Append()variants do not have that problem since they use an NCTableLine that was created by the caller, and NCTableLine has its own_indexmember variable that is set (typically from the corresponding YItem) in its constructor.Test
Use this new UI example and change the "Vegetable" check box. Watch the output fields and the y2log.
https://github.com/shundhammer/yast-ycp-ui-bindings/blob/huha-multisel-01/examples/MultiSelectionBox-test.rb
Automated Test
An automated test case based on the above test will follow.