Page MenuHomePhabricator

edje_object_signal_callback_add issue using layout as source
Closed, ResolvedPublic

Description

after commit 917e0aa0feb9a79721953ea632723d26a251a850

i cant use "layout" as valid Evas_Object handle for edje_object_signal_callback_add.
it is not working anymore.

layout ist added with

Evas_Object *ly = elm_layout_add(win);

i have to add

Evas_Object *edje_obj = elm_layout_edje_get(ly);

before i can use

edje_object_signal_callback_add(edje_obj, "show_popup", "show_popup", show_popup, win);

before the changes i was able to use the "layout" directly as valid Evas_Object handle. the following code was working correctly

edje_object_signal_callback_add(ly, "show_popup", "show_popup", show_popup, win);

here the code working correctly before the commit https://github.com/jf-simon/news/blob/master/src/main.c#L1269

thanks for looking at this issue

Greetings Simon

jf_simon created this task.Feb 7 2019, 12:28 AM
cedric triaged this task as Showstopper Issues priority.Feb 7 2019, 1:15 AM
cedric added subscribers: stefan_schmidt, zmike, cedric.

Oh, I see, hum, I guess we should actually restore this behavior as this is legacy. Interesting that the efl interface does have this kind of side effect.

zmike added a comment.Feb 7 2019, 10:39 AM

Oh, I see, hum, I guess we should actually restore this behavior as this is legacy. Interesting that the efl interface does have this kind of side effect.

I'm not so sure that's a great idea. There are two apis which should have been used for this case: elm_layout_signal_callback_add and elm_object_signal_callback_add. edje_object_signal_callback_add has intermittently worked across releases, but this is not a documented or (imo) expected behavior. While we do try to preserve legacy behavior, that's only the case for intended and "not obviously wrong" behavior. I think this is a case where it's obviously wrong given that the use case is calling an api from one object onto another unrelated object.

Furthermore, fixing this such that it also works as expected with signal deletion (i.e., returning the user data) would require a substantive amount of spaghetti code.

zmike added a comment.Feb 8 2019, 10:47 AM

fwiw I started looking into this; here's roughly the code required to mimic previous edje_object_signal_callback_add functionality. The code for removing signal callbacks, however, would be crazy spaghetti code since it needs to return the user data.

diff --git a/src/lib/edje/edje_legacy.c b/src/lib/edje/edje_legacy.c
index fead8b04be..756611ea62 100644
--- a/src/lib/edje/edje_legacy.c
+++ b/src/lib/edje/edje_legacy.c
@@ -74,13 +74,38 @@ edje_object_message_signal_recursive_process(Edje_Object *obj)
    efl_layout_signal_process(obj, EINA_TRUE);
 }
 
+typedef struct
+{
+   void *data;
+   Edje_Signal_Cb cb;
+   Evas_Object *obj;
+} legacy_layout_data;
+
+static void
+_legacy_layout_signal_cb(void *data, Efl_Layout_Signal *object, const char *emission, const char *source)
+{
+   legacy_layout_data *ld = data;
+
+   ld->cb(ld->data, ld->obj, emission, source);
+}
+
 EAPI void
 edje_object_signal_callback_add(Evas_Object *obj, const char *emission, const char *source, Edje_Signal_Cb func, void *data)
 {
    Edje *ed;
 
    ed = _edje_fetch(obj);
-   if (!ed || ed->delete_me) return;
+   if (!ed) /* compat for elm_layout */
+     {
+        legacy_layout_data *ld = malloc(sizeof(legacy_layout_data));
+        EINA_SAFETY_ON_NULL_RETURN(ld);
+        ld->data = data;
+        ld->cb = func;
+        ld->obj = obj;
+        efl_layout_signal_callback_add(obj, emission, source, ld, _legacy_layout_signal_cb, free);
+        return;
+     }
+   if (ed->delete_me) return;
    _edje_object_signal_callback_add(ed, emission, source, func, NULL, NULL, data);
 }
bu5hm4n closed this task as Resolved.Mar 30 2019, 4:51 AM

This is resolved afair on @jf_simon side.