aboutsummaryrefslogtreecommitdiffstats
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
parentgenlist: Simplify some logic (diff)
downloadefl-devs/jpeg/genlist_crazy.tar.gz
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)
return obj;
}
+EOLIAN static Eo *
+_elm_genlist_efl_object_finalize(Eo *obj, Elm_Genlist_Data *sd)
+{
+ obj = efl_finalize(efl_super(obj, MY_CLASS));
+
+ // FIXME:
+ // Genlist is a legacy-only widget which means it will always be marked as
+ // "legacy" here. The proper test here is "app targets EFL 1.19 or below".
+ if (elm_widget_is_legacy(obj))
+ sd->legacy_order_insane = EINA_TRUE;
+
+ return obj;
+}
+
static void
_internal_elm_genlist_clear(Evas_Object *obj)
{
@@ -6704,10 +6718,198 @@ _elm_genlist_at_xy_item_get(const Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd, Eva
return NULL;
}
+/* ======== Genlist legacy item ordering fix. Insanity begins here. ========
+ *
+ * Welcome to insanity land, where items' real order is not what matters.
+ *
+ * Sane order:
+ *
+ * parent1
+ * parent2
+ * leaf1
+ * leaf2
+ * leaf3
+ * parent3
+ * leaf4
+ * leaf5
+ *
+ * Legacy "insane" order:
+ *
+ * leaf1
+ * leaf2
+ * parent2
+ * leaf3
+ * leaf4
+ * leaf5
+ * parent3
+ * parent1
+ *
+ * Then filtering comes into play. Fun times.
+ * The implementation has undefined behavior. Don't worry. This is already
+ * insane anyway.
+ *
+ */
+
+static inline Elm_Gen_Item *
+_insane_item_unfiltered_child_find(Eina_Bool filter, Elm_Gen_Item *it_parent)
+{
+ Eo *eo_it = NULL;
+ Elm_Gen_Item *it, *it2;
+ Eina_List *li;
+
+ EINA_SAFETY_ON_NULL_RETURN_VAL(it_parent->item, NULL);
+ EINA_LIST_FOREACH(it_parent->item->items, li, eo_it)
+ {
+ it = efl_data_scope_get(eo_it, ELM_GENLIST_ITEM_CLASS);
+ if (!filter || _item_filtered_get(it))
+ {
+ it2 = _insane_item_unfiltered_child_find(filter, it);
+ if (it2) return it2;
+ else return it;
+ }
+ }
+
+ if (!eo_it) return NULL;
+ return it_parent;
+}
+
+static Elm_Object_Item *
+_elm_genlist_first_item_get_insane(Elm_Genlist_Data *sd)
+{
+ Elm_Gen_Item *it, *it2;
+
+ it = ELM_GEN_ITEM_FROM_INLIST(sd->items);
+ if (!it) return NULL;
+
+ // Find first unfiltered item (sane order)
+ while (it)
+ {
+ if (!sd->filter || _item_filtered_get(it)) break;
+ it = ELM_GEN_ITEM_NEXT(it);
+ }
+ if (!it) return NULL;
+
+ // Find a "leaf" item that isn't filtered...
+ it2 = _insane_item_unfiltered_child_find(sd->filter, it);
+ if (it2) it = it2;
+
+ return EO_OBJ(it);
+}
+
+static Elm_Object_Item *
+_elm_genlist_last_item_get_insane(Elm_Genlist_Data *sd)
+{
+ Elm_Gen_Item *it;
+
+ if (!sd->items) return NULL;
+
+ // Find last unfiltered item (sane order)
+ it = ELM_GEN_ITEM_FROM_INLIST(sd->items->last);
+ while (it)
+ {
+ if (!sd->filter || _item_filtered_get(it)) break;
+ it = ELM_GEN_ITEM_PREV(it);
+ }
+ if (!it) return NULL;
+
+ // Parents are not filtered out and are after the items, in insane order.
+ while (it->parent)
+ it = it->parent;
+
+ return EO_OBJ(it);
+}
+
+static Elm_Object_Item *
+_elm_genlist_next_item_get_insane(Elm_Genlist_Data *sd, Elm_Gen_Item *it)
+{
+ Elm_Gen_Item *it2;
+
+ if (sd->filter && !_item_filtered_get(it))
+ {
+ // If this item is filtered out, give up on smarts and just find
+ // the next filtered one.
+ for (it2 = ELM_GEN_ITEM_NEXT(it); it2; it2 = ELM_GEN_ITEM_NEXT(it2))
+ if (!sd->filter || _item_filtered_get(it2))
+ break;
+ return EO_OBJ(it2);
+ }
+
+ for (it2 = ELM_GEN_ITEM_NEXT(it); it2; it2 = ELM_GEN_ITEM_NEXT(it2))
+ {
+ // 0. If filtered out, go to next
+ if (sd->filter && !_item_filtered_get(it2))
+ continue;
+
+ // 1. Return next sibling in list, if any
+ if (it->parent == it2->parent)
+ return EO_OBJ(it2);
+
+ // 2. If next item is a child, skip until next sibling
+ if (it2->parent == it)
+ {
+ Eo *eo_it2 = NULL;
+ Eina_List *last;
+
+ EINA_SAFETY_ON_NULL_RETURN_VAL(it->item, NULL);
+ last = _list_last_recursive(it->item->items);
+ if (last) eo_it2 = last->data;
+ it2 = efl_data_scope_get(eo_it2, ELM_GENLIST_ITEM_CLASS);
+ continue;
+ }
+
+ // 3. If next item has another parent (or NULL parent), return it's parent
+ // Note: it's parent can't be filtered out, unless it was also filtered out.
+ if (it->parent && (it->parent != it2->parent))
+ return EO_OBJ(it->parent);
+ }
+
+ return EO_OBJ(it2);
+}
+
+static Elm_Object_Item *
+_elm_genlist_prev_item_get_insane(Elm_Genlist_Data *sd, Elm_Gen_Item *it)
+{
+ Elm_Gen_Item *it2, *parent;
+
+ if (sd->filter && !_item_filtered_get(it))
+ {
+ // If this item is filtered out, give up on smarts and just find
+ // the previous filtered one.
+ for (it2 = ELM_GEN_ITEM_PREV(it); it2; it2 = ELM_GEN_ITEM_PREV(it2))
+ if (!sd->filter || _item_filtered_get(it2))
+ break;
+ return EO_OBJ(it2);
+ }
+
+ parent = it->parent;
+ if (!parent)
+ {
+ // No parent: just return previous filtered item we can find.
+ for (it2 = ELM_GEN_ITEM_PREV(it); it2; it2 = ELM_GEN_ITEM_PREV(it2))
+ if (!sd->filter || _item_filtered_get(it2))
+ break;
+ return EO_OBJ(it2);
+ }
+
+ it2 = ELM_GEN_ITEM_PREV(it);
+ if (it2 == parent)
+ return _elm_genlist_prev_item_get_insane(sd, it2);
+
+ return EO_OBJ(it2);
+}
+
+/* ======== Genlist legacy item ordering fix. Insanity ends here. ========== */
+
+
EOLIAN static Elm_Object_Item*
_elm_genlist_first_item_get(Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd)
{
- Elm_Gen_Item *it = ELM_GEN_ITEM_FROM_INLIST(sd->items);
+ Elm_Gen_Item *it;
+
+ if (EINA_LIKELY(sd->legacy_order_insane))
+ return _elm_genlist_first_item_get_insane(sd);
+
+ it = ELM_GEN_ITEM_FROM_INLIST(sd->items);
while (it && sd->filter && !_item_filtered_get(it))
it = ELM_GEN_ITEM_NEXT(it);
@@ -6720,6 +6922,9 @@ _elm_genlist_last_item_get(Eo *obj EINA_UNUSED, Elm_Genlist_Data *sd)
{
Elm_Gen_Item *it;
+ if (EINA_LIKELY(sd->legacy_order_insane))
+ return _elm_genlist_last_item_get_insane(sd);
+
if (!sd->items) return NULL;
it = ELM_GEN_ITEM_FROM_INLIST(sd->items->last);
@@ -6734,6 +6939,9 @@ _elm_genlist_item_next_get(Eo *eo_it EINA_UNUSED, Elm_Gen_Item *it)
{
ELM_GENLIST_DATA_GET_FROM_ITEM(it, sd);
+ if (EINA_LIKELY(sd->legacy_order_insane))
+ return _elm_genlist_next_item_get_insane(sd, it);
+
do it = ELM_GEN_ITEM_NEXT(it);
while (it && sd->filter && !_item_filtered_get(it));
@@ -6745,6 +6953,9 @@ _elm_genlist_item_prev_get(Eo *eo_it EINA_UNUSED, Elm_Gen_Item *it)
{
ELM_GENLIST_DATA_GET_FROM_ITEM(it, sd);
+ if (EINA_LIKELY(sd->legacy_order_insane))
+ return _elm_genlist_prev_item_get_insane(sd, it);
+
do it = ELM_GEN_ITEM_PREV(it);
while (it && sd->filter && !_item_filtered_get(it));
@@ -7599,6 +7810,7 @@ _filter_item_internal(Elm_Gen_Item *it)
}
// Returns true if the item is not filtered out, but remains visible.
+// If a parent is filtered out its children are also filtered out.
static Eina_Bool
_item_filtered_get(Elm_Gen_Item *it)
{
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,
implements {
class.constructor;
Efl.Object.constructor;
+ Efl.Object.finalize;
Efl.Gfx.position { set; }
Efl.Gfx.size { set; }
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
Eina_Bool item_looping_on : 1;
Eina_Bool tree_effect_animator : 1;
+
+ // If true, use insane legacy item order: item, item, parent
+ Eina_Bool legacy_order_insane : 1;
};
typedef struct _Item_Block Item_Block;