summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJean-Philippe Andre <jp.andre@samsung.com>2017-10-24 18:09:50 +0900
committerJean-Philippe Andre <jp.andre@samsung.com>2017-10-25 22:56:57 +0900
commit2ed591a9a987e07745f2452ce119433de91abc67 (patch)
tree213329a1d5aa3b28deee2d6b6bedc26871ec643d
parent585be9e24f10d683b955be43a31632ac7067cbd2 (diff)
genlist: Fix "insane" order [BUG COMPATIBILITY]devs/jpeg/genlist_crazy
This patch implements bug compatibility. genlist internally uses both a tree structure with Eina_List and a flat Eina_Inlist to track its items. ALL of the items are in the inlist while subitems appear in their parent's list. As a consequence both lists must be kept in sync pretty tightly. Obviously this is not done at all and has led to countless bugs, as soon as tree or groups are used: - Invalid order of items (visually) - Invalid order of items with sorted_insert - Glitches in the matrix - Crashes with sorted_insert - Odd/even styles not properly set - Promote/demote functions broken by design - Developers send to psychiatric hospitals - Etc... Legacy genlist (1.19 and before) used an inlist order that basically didn't make sense, as it didn't follow the logical order of elements (as they appear visually). Unfortunately this has "worked" (really, that's a huge stretch to use this word here) for a long time this way. As a consequence, some applications (*cough* empc *cough*) have relied on this order to implement "next album" or "previous album" where the album title is a group node. By changing the order of items in the inlist, this has broken the assumptions made above, and ends up in cases that return NULL, leading to SEGV. Sure, the app should have checked NULL, but that's not really the point here. The behavior has been changed. This patch implements "fixes" for the following functions: - elm_genlist_first_item_get(): Don't return a parent - elm_genlist_last_item_get(): Return a parent Important notes: - This does not cover 100% behavior compatibility here. The only way to have it would be to simply revert the entire genlist code to its original version and never touch it again, ever. - An explicit API is required for an application to specify which API level it targets, so that we can cherry-pick which bug compatibility features we want to enable. We are already doing this for EDC, unfortunately. @fix
-rw-r--r--src/lib/elementary/elm_genlist.c214
-rw-r--r--src/lib/elementary/elm_genlist.eo1
-rw-r--r--src/lib/elementary/elm_widget_genlist.h3
3 files changed, 217 insertions, 1 deletions
diff --git a/src/lib/elementary/elm_genlist.c b/src/lib/elementary/elm_genlist.c
index a0706abc2b..105fba27e8 100644
--- a/src/lib/elementary/elm_genlist.c
+++ b/src/lib/elementary/elm_genlist.c
@@ -5729,6 +5729,20 @@ _elm_genlist_efl_object_constructor(Eo *obj, Elm_Genlist_Data *sd)
5729 return obj; 5729 return obj;
5730} 5730}
5731 5731
5732EOLIAN static Eo *
5733_elm_genlist_efl_object_finalize(Eo *obj, Elm_Genlist_Data *sd)
5734{
5735 obj = efl_finalize(efl_super(obj, MY_CLASS));
5736
5737 // FIXME:
5738 // Genlist is a legacy-only widget which means it will always be marked as
5739 // "legacy" here. The proper test here is "app targets EFL 1.19 or below".
5740 if (elm_widget_is_legacy(obj))
5741 sd->legacy_order_insane = EINA_TRUE;
5742
5743 return obj;
5744}
5745
5732static void 5746static void
5733_internal_elm_genlist_clear(Evas_Object *obj) 5747_internal_elm_genlist_clear(Evas_Object *obj)
5734{ 5748{
@@ -6704,10 +6718,198 @@ _elm_genlist_at_xy_item_get(const Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd, Eva
6704 return NULL; 6718 return NULL;
6705} 6719}
6706 6720
6721/* ======== Genlist legacy item ordering fix. Insanity begins here. ========
6722 *
6723 * Welcome to insanity land, where items' real order is not what matters.
6724 *
6725 * Sane order:
6726 *
6727 * parent1
6728 * parent2
6729 * leaf1
6730 * leaf2
6731 * leaf3
6732 * parent3
6733 * leaf4
6734 * leaf5
6735 *
6736 * Legacy "insane" order:
6737 *
6738 * leaf1
6739 * leaf2
6740 * parent2
6741 * leaf3
6742 * leaf4
6743 * leaf5
6744 * parent3
6745 * parent1
6746 *
6747 * Then filtering comes into play. Fun times.
6748 * The implementation has undefined behavior. Don't worry. This is already
6749 * insane anyway.
6750 *
6751 */
6752
6753static inline Elm_Gen_Item *
6754_insane_item_unfiltered_child_find(Eina_Bool filter, Elm_Gen_Item *it_parent)
6755{
6756 Eo *eo_it = NULL;
6757 Elm_Gen_Item *it, *it2;
6758 Eina_List *li;
6759
6760 EINA_SAFETY_ON_NULL_RETURN_VAL(it_parent->item, NULL);
6761 EINA_LIST_FOREACH(it_parent->item->items, li, eo_it)
6762 {
6763 it = efl_data_scope_get(eo_it, ELM_GENLIST_ITEM_CLASS);
6764 if (!filter || _item_filtered_get(it))
6765 {
6766 it2 = _insane_item_unfiltered_child_find(filter, it);
6767 if (it2) return it2;
6768 else return it;
6769 }
6770 }
6771
6772 if (!eo_it) return NULL;
6773 return it_parent;
6774}
6775
6776static Elm_Object_Item *
6777_elm_genlist_first_item_get_insane(Elm_Genlist_Data *sd)
6778{
6779 Elm_Gen_Item *it, *it2;
6780
6781 it = ELM_GEN_ITEM_FROM_INLIST(sd->items);
6782 if (!it) return NULL;
6783
6784 // Find first unfiltered item (sane order)
6785 while (it)
6786 {
6787 if (!sd->filter || _item_filtered_get(it)) break;
6788 it = ELM_GEN_ITEM_NEXT(it);
6789 }
6790 if (!it) return NULL;
6791
6792 // Find a "leaf" item that isn't filtered...
6793 it2 = _insane_item_unfiltered_child_find(sd->filter, it);
6794 if (it2) it = it2;
6795
6796 return EO_OBJ(it);
6797}
6798
6799static Elm_Object_Item *
6800_elm_genlist_last_item_get_insane(Elm_Genlist_Data *sd)
6801{
6802 Elm_Gen_Item *it;
6803
6804 if (!sd->items) return NULL;
6805
6806 // Find last unfiltered item (sane order)
6807 it = ELM_GEN_ITEM_FROM_INLIST(sd->items->last);
6808 while (it)
6809 {
6810 if (!sd->filter || _item_filtered_get(it)) break;
6811 it = ELM_GEN_ITEM_PREV(it);
6812 }
6813 if (!it) return NULL;
6814
6815 // Parents are not filtered out and are after the items, in insane order.
6816 while (it->parent)
6817 it = it->parent;
6818
6819 return EO_OBJ(it);
6820}
6821
6822static Elm_Object_Item *
6823_elm_genlist_next_item_get_insane(Elm_Genlist_Data *sd, Elm_Gen_Item *it)
6824{
6825 Elm_Gen_Item *it2;
6826
6827 if (sd->filter && !_item_filtered_get(it))
6828 {
6829 // If this item is filtered out, give up on smarts and just find
6830 // the next filtered one.
6831 for (it2 = ELM_GEN_ITEM_NEXT(it); it2; it2 = ELM_GEN_ITEM_NEXT(it2))
6832 if (!sd->filter || _item_filtered_get(it2))
6833 break;
6834 return EO_OBJ(it2);
6835 }
6836
6837 for (it2 = ELM_GEN_ITEM_NEXT(it); it2; it2 = ELM_GEN_ITEM_NEXT(it2))
6838 {
6839 // 0. If filtered out, go to next
6840 if (sd->filter && !_item_filtered_get(it2))
6841 continue;
6842
6843 // 1. Return next sibling in list, if any
6844 if (it->parent == it2->parent)
6845 return EO_OBJ(it2);
6846
6847 // 2. If next item is a child, skip until next sibling
6848 if (it2->parent == it)
6849 {
6850 Eo *eo_it2 = NULL;
6851 Eina_List *last;
6852
6853 EINA_SAFETY_ON_NULL_RETURN_VAL(it->item, NULL);
6854 last = _list_last_recursive(it->item->items);
6855 if (last) eo_it2 = last->data;
6856 it2 = efl_data_scope_get(eo_it2, ELM_GENLIST_ITEM_CLASS);
6857 continue;
6858 }
6859
6860 // 3. If next item has another parent (or NULL parent), return it's parent
6861 // Note: it's parent can't be filtered out, unless it was also filtered out.
6862 if (it->parent && (it->parent != it2->parent))
6863 return EO_OBJ(it->parent);
6864 }
6865
6866 return EO_OBJ(it2);
6867}
6868
6869static Elm_Object_Item *
6870_elm_genlist_prev_item_get_insane(Elm_Genlist_Data *sd, Elm_Gen_Item *it)
6871{
6872 Elm_Gen_Item *it2, *parent;
6873
6874 if (sd->filter && !_item_filtered_get(it))
6875 {
6876 // If this item is filtered out, give up on smarts and just find
6877 // the previous filtered one.
6878 for (it2 = ELM_GEN_ITEM_PREV(it); it2; it2 = ELM_GEN_ITEM_PREV(it2))
6879 if (!sd->filter || _item_filtered_get(it2))
6880 break;
6881 return EO_OBJ(it2);
6882 }
6883
6884 parent = it->parent;
6885 if (!parent)
6886 {
6887 // No parent: just return previous filtered item we can find.
6888 for (it2 = ELM_GEN_ITEM_PREV(it); it2; it2 = ELM_GEN_ITEM_PREV(it2))
6889 if (!sd->filter || _item_filtered_get(it2))
6890 break;
6891 return EO_OBJ(it2);
6892 }
6893
6894 it2 = ELM_GEN_ITEM_PREV(it);
6895 if (it2 == parent)
6896 return _elm_genlist_prev_item_get_insane(sd, it2);
6897
6898 return EO_OBJ(it2);
6899}
6900
6901/* ======== Genlist legacy item ordering fix. Insanity ends here. ========== */
6902
6903
6707EOLIAN static Elm_Object_Item* 6904EOLIAN static Elm_Object_Item*
6708_elm_genlist_first_item_get(Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd) 6905_elm_genlist_first_item_get(Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd)
6709{ 6906{
6710 Elm_Gen_Item *it = ELM_GEN_ITEM_FROM_INLIST(sd->items); 6907 Elm_Gen_Item *it;
6908
6909 if (EINA_LIKELY(sd->legacy_order_insane))
6910 return _elm_genlist_first_item_get_insane(sd);
6911
6912 it = ELM_GEN_ITEM_FROM_INLIST(sd->items);
6711 6913
6712 while (it && sd->filter && !_item_filtered_get(it)) 6914 while (it && sd->filter && !_item_filtered_get(it))
6713 it = ELM_GEN_ITEM_NEXT(it); 6915 it = ELM_GEN_ITEM_NEXT(it);
@@ -6720,6 +6922,9 @@ _elm_genlist_last_item_get(Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd)
6720{ 6922{
6721 Elm_Gen_Item *it; 6923 Elm_Gen_Item *it;
6722 6924
6925 if (EINA_LIKELY(sd->legacy_order_insane))
6926 return _elm_genlist_last_item_get_insane(sd);
6927
6723 if (!sd->items) return NULL; 6928 if (!sd->items) return NULL;
6724 it = ELM_GEN_ITEM_FROM_INLIST(sd->items->last); 6929 it = ELM_GEN_ITEM_FROM_INLIST(sd->items->last);
6725 6930
@@ -6734,6 +6939,9 @@ _elm_genlist_item_next_get(Eo *eo_it EINA_UNUSED, Elm_Gen_Item *it)
6734{ 6939{
6735 ELM_GENLIST_DATA_GET_FROM_ITEM(it, sd); 6940 ELM_GENLIST_DATA_GET_FROM_ITEM(it, sd);
6736 6941
6942 if (EINA_LIKELY(sd->legacy_order_insane))
6943 return _elm_genlist_next_item_get_insane(sd, it);
6944
6737 do it = ELM_GEN_ITEM_NEXT(it); 6945 do it = ELM_GEN_ITEM_NEXT(it);
6738 while (it && sd->filter && !_item_filtered_get(it)); 6946 while (it && sd->filter && !_item_filtered_get(it));
6739 6947
@@ -6745,6 +6953,9 @@ _elm_genlist_item_prev_get(Eo *eo_it EINA_UNUSED, Elm_Gen_Item *it)
6745{ 6953{
6746 ELM_GENLIST_DATA_GET_FROM_ITEM(it, sd); 6954 ELM_GENLIST_DATA_GET_FROM_ITEM(it, sd);
6747 6955
6956 if (EINA_LIKELY(sd->legacy_order_insane))
6957 return _elm_genlist_prev_item_get_insane(sd, it);
6958
6748 do it = ELM_GEN_ITEM_PREV(it); 6959 do it = ELM_GEN_ITEM_PREV(it);
6749 while (it && sd->filter && !_item_filtered_get(it)); 6960 while (it && sd->filter && !_item_filtered_get(it));
6750 6961
@@ -7599,6 +7810,7 @@ _filter_item_internal(Elm_Gen_Item *it)
7599} 7810}
7600 7811
7601// Returns true if the item is not filtered out, but remains visible. 7812// Returns true if the item is not filtered out, but remains visible.
7813// If a parent is filtered out its children are also filtered out.
7602static Eina_Bool 7814static Eina_Bool
7603_item_filtered_get(Elm_Gen_Item *it) 7815_item_filtered_get(Elm_Gen_Item *it)
7604{ 7816{
diff --git a/src/lib/elementary/elm_genlist.eo b/src/lib/elementary/elm_genlist.eo
index 3a6969e6f3..6eb88d13b8 100644
--- a/src/lib/elementary/elm_genlist.eo
+++ b/src/lib/elementary/elm_genlist.eo
@@ -529,6 +529,7 @@ class Elm.Genlist (Efl.Ui.Layout, Elm.Interface_Scrollable, Efl.Ui.Clickable,
529 implements { 529 implements {
530 class.constructor; 530 class.constructor;
531 Efl.Object.constructor; 531 Efl.Object.constructor;
532 Efl.Object.finalize;
532 Efl.Gfx.position { set; } 533 Efl.Gfx.position { set; }
533 Efl.Gfx.size { set; } 534 Efl.Gfx.size { set; }
534 Efl.Canvas.Group.group_member_add; 535 Efl.Canvas.Group.group_member_add;
diff --git a/src/lib/elementary/elm_widget_genlist.h b/src/lib/elementary/elm_widget_genlist.h
index 5a1f4e8008..a35e93ce74 100644
--- a/src/lib/elementary/elm_widget_genlist.h
+++ b/src/lib/elementary/elm_widget_genlist.h
@@ -201,6 +201,9 @@ struct _Elm_Genlist_Data
201 Eina_Bool item_looping_on : 1; 201 Eina_Bool item_looping_on : 1;
202 202
203 Eina_Bool tree_effect_animator : 1; 203 Eina_Bool tree_effect_animator : 1;
204
205 // If true, use insane legacy item order: item, item, parent
206 Eina_Bool legacy_order_insane : 1;
204}; 207};
205 208
206typedef struct _Item_Block Item_Block; 209typedef struct _Item_Block Item_Block;