HomePhabricator

genlist: fix "insane" order [BUG COMPATIBILITY]

Authored by Jean-Philippe Andre <jp.andre@samsung.com> on Oct 24 2017, 2:09 AM.

Description

genlist: fix "insane" order [BUG COMPATIBILITY]

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
  • elm_genlist_item_next_get(): return a parent upon reaching the last child
  • elm_genlist_item_prev_get(): return a child when a parent is passed

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

fix T5938

Signed-off-by: Mike Blumenkrantz <zmike@osg.samsung.com>
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>