After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752615 - listbox: Model items changed doesn't take sorting into account
listbox: Model items changed doesn't take sorting into account
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-07-20 08:51 UTC by Adrien Plazas
Modified: 2015-08-27 03:34 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Adrien Plazas 2015-07-20 08:51:13 UTC
When items are removed from the model bound to a GtkListBox, the rows are removed with no consideration to the sorting, which lead to wrong rows being removed.

Example:

The model contains the elements "1", "2" and "3" in that order. The sorting function of the listbox order them as "3", "2" and "1".

When the 3rd element of the model if removed (so that the model is "1" and "2", the listbox just remove it's 3rd element, letting "3" and "2".

Updating gtk_list_box_bound_model_changed() to take sorting into account could fix it. This function also seems no to take filtering into account.
Comment 1 Adrien Plazas 2015-07-20 08:54:13 UTC
Disabling the sorting and filtering before updating the rows, then re-enabling these would be a quick and dirty fix.
Comment 2 Matthias Clasen 2015-07-23 05:25:50 UTC
I believe that the idea with gtk_list_box_set_model is that the model would do the sorting and filtering, and you don't use GtkListBox' support for that.
Comment 3 Adrien Plazas 2015-07-23 07:41:01 UTC
If it is the case then the GtkListBox shouldn't react to the model's "items-changed" signal, as it is currently buggy.

Also, there is currently no GListModel offering sorting and filtering capabilities, but GtkListBox and GtkFlowBox do. That being said, GtkFlowBox can't be bound to a model.

Being able to bind a model and filtering and sorting functions to the GtkListBox without being able to bind at the same time a model and one of the functions is weird.

If fixing the bug is not an option, maybe deprecating the binding method to clearly state that it shouldn't be used and to match GtkFlowBox's API could do the trick?
Comment 4 Matthias Clasen 2015-07-23 11:41:42 UTC
You really need Lars to chime in here, who has done all of the list model work.
Comment 5 Zeeshan Ali 2015-08-24 14:14:52 UTC
This bug meant some not so nice workaround in Boxes. :(
Comment 6 Lars Karlitski 2015-08-24 18:21:31 UTC
In general I agree with Matthias - the model should take care of sorting and filtering, because only it knows how to sort and filter its data efficiently.

That said, it is kind of awkward that filtering works with bound models (hiding widgets doesn't change indices) but sorting doesn't. Also, performance benefits for many models are moot anyway, because GtkListBox always fetches all items.

It shouldn't be too hard to map indices from the model to sorted ones from the box. It wouldn't be terribly fast, but people are hopefully only using all if this with only a few items.

Alternatively, we could document that setting a sort func doesn't work when binding the box to a model and throw a critical when somebody does that.

Note: GListStore does have support for sorting through g_list_store_insert_sorted() (Not filtering though).
Comment 7 Adrien Plazas 2015-08-24 18:51:05 UTC
What about deprecating gtk_list_box_bind_model?

It would:
- make GtkListBox's API closer to GtkFlowBox's one,
- remove the parts of the API you don't like,
- make clear that this method shouldn't be used,
- make you not have to actually fix the bug. =p
Comment 8 Emmanuele Bassi (:ebassi) 2015-08-24 19:51:15 UTC
(In reply to Adrien Plazas from comment #7)
> What about deprecating gtk_list_box_bind_model?
> 
> It would:
> - make GtkListBox's API closer to GtkFlowBox's one,

GtkFlowBox has a bind_model() method in 3.17.

> - remove the parts of the API you don't like,

Considering that Lars was the person that added the method in the first place, that does not sound like a good idea.

> - make clear that this method shouldn't be used,
> - make you not have to actually fix the bug. =p

That does not sound like an acceptable solution at all.

I'd rather document the sorting/filtering functions in GtkListBox and GtkFlowBox as either deprecated, or not working with bound models, and have the models provide sorting and filtering.
Comment 9 Matthias Clasen 2015-08-25 21:43:50 UTC
I've added documentation for the incompatibility of models and sorting/filtering, and we print a warning now if that combination is seen.
Comment 10 Matthias Clasen 2015-08-27 03:34:09 UTC
Also see bug 754152