summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Zaoui <daniel.zaoui@samsung.com>2015-05-22 10:07:20 +0300
committerThiep Ha <thiepha@gmail.com>2015-06-02 21:50:25 +0900
commita3a225186191591e99c6c6d186c1a1de26b65546 (patch)
treeeceecb8f7812ebb6443d2924f36415d6647417d6
parent15be2879daf82df05e24ff84e34aaaa1025851f5 (diff)
First review
Hi Thiep, I finally had time to review dnd. I put my comments in this patch. One important thing is that we don't have to see the X11 implementation as a strong reference. A lot of code is shitty there and we should throw it on both implementations. I didn't review all the code as I think there is a lot to fix already and open issues. Concerning the notify stuff, I don't think we should conserve the X11 way (i.e call cb inside notify or entry/image functions). We could, for example, create data preparers that would be invoked before cb is called. Daniel
-rw-r--r--src/lib/elm_cnp.c23
1 files changed, 22 insertions, 1 deletions
diff --git a/src/lib/elm_cnp.c b/src/lib/elm_cnp.c
index 7be51dab0..afc2c9328 100644
--- a/src/lib/elm_cnp.c
+++ b/src/lib/elm_cnp.c
@@ -2562,7 +2562,7 @@ _wl_targets_converter(char *target, Wl_Cnp_Selection *sel EINA_UNUSED, void *dat
2562 if (sel->format) 2562 if (sel->format)
2563 { 2563 {
2564 formats = sel->format; 2564 formats = sel->format;
2565 is_uri_data = _wl_is_uri_type_data(sel->selbuf, sel->buflen); 2565 is_uri_data = _wl_is_uri_type_data(sel->selbuf, sel->buflen); /* why? */
2566 } 2566 }
2567 else 2567 else
2568 { 2568 {
@@ -2575,6 +2575,8 @@ _wl_targets_converter(char *target, Wl_Cnp_Selection *sel EINA_UNUSED, void *dat
2575 { 2575 {
2576 if (formats & _atoms[i].formats) 2576 if (formats & _atoms[i].formats)
2577 { 2577 {
2578 /* Why do we need only uri?
2579 * If really needed, instead of strcmp why not just compare atom ids? */
2578 if ((is_uri_data) || (!is_uri_data && 2580 if ((is_uri_data) || (!is_uri_data &&
2579 strcmp(_atoms[i].name, "text/uri") && 2581 strcmp(_atoms[i].name, "text/uri") &&
2580 strcmp(_atoms[i].name, "text/uri-list"))) 2582 strcmp(_atoms[i].name, "text/uri-list")))
@@ -2587,6 +2589,7 @@ _wl_targets_converter(char *target, Wl_Cnp_Selection *sel EINA_UNUSED, void *dat
2587 { 2589 {
2588 if (formats & _atoms[i].formats) 2590 if (formats & _atoms[i].formats)
2589 { 2591 {
2592 /* Same here */
2590 if ((is_uri_data) || (!is_uri_data && 2593 if ((is_uri_data) || (!is_uri_data &&
2591 strcmp(_atoms[i].name, "text/uri") && 2594 strcmp(_atoms[i].name, "text/uri") &&
2592 strcmp(_atoms[i].name, "text/uri-list"))) 2595 strcmp(_atoms[i].name, "text/uri-list")))
@@ -2816,6 +2819,10 @@ _wl_notify_handler_uri(Wl_Cnp_Selection *sel, Ecore_Wl_Event_Selection_Data_Read
2816 Eina_Inlist *itr; 2819 Eina_Inlist *itr;
2817 Elm_Selection_Data ddata; 2820 Elm_Selection_Data ddata;
2818 2821
2822 /* Is this function only needed for DnD?
2823 * Because it seems like, as __elm_dropable is set only when a drop target
2824 * is added! If this function is called during selection, it will sigsegv!
2825 */
2819 eo_do(sel->requestwidget, drop = eo_key_data_get("__elm_dropable")); 2826 eo_do(sel->requestwidget, drop = eo_key_data_get("__elm_dropable"));
2820 if (drop) 2827 if (drop)
2821 type = drop->last.type; 2828 type = drop->last.type;
@@ -2842,6 +2849,9 @@ _wl_notify_handler_uri(Wl_Cnp_Selection *sel, Ecore_Wl_Event_Selection_Data_Read
2842 uri = calloc(1, sizeof(*uri) * num_files); 2849 uri = calloc(1, sizeof(*uri) * num_files);
2843 if (!uri) return 0; 2850 if (!uri) return 0;
2844 2851
2852 /* Can't it be easier with Eina_Strbuf?
2853 * Seems complicated for nothing.
2854 */
2845 for (i = 0; i < num_files ; i++) 2855 for (i = 0; i < num_files ; i++)
2846 { 2856 {
2847 uri[i] = efreet_uri_decode(files[i]); 2857 uri[i] = efreet_uri_decode(files[i]);
@@ -2967,6 +2977,7 @@ _wl_notify_handler_uri(Wl_Cnp_Selection *sel, Ecore_Wl_Event_Selection_Data_Read
2967 2977
2968 mkupstr = _elm_util_text_to_mkup((const char *)stripstr); 2978 mkupstr = _elm_util_text_to_mkup((const char *)stripstr);
2969 /* TODO BUG: should never NEVER assume it's an elm_entry! */ 2979 /* TODO BUG: should never NEVER assume it's an elm_entry! */
2980 /* This should not be here! */
2970 _elm_entry_entry_paste(sel->requestwidget, mkupstr); 2981 _elm_entry_entry_paste(sel->requestwidget, mkupstr);
2971 free(mkupstr); 2982 free(mkupstr);
2972 } 2983 }
@@ -3197,6 +3208,10 @@ static Eina_Bool _wl_drops_accept(const char *type);
3197static unsigned int _wl_elm_widget_window_get(Evas_Object *obj); 3208static unsigned int _wl_elm_widget_window_get(Evas_Object *obj);
3198static Evas * _wl_evas_get_from_win(unsigned int win); 3209static Evas * _wl_evas_get_from_win(unsigned int win);
3199 3210
3211/* I think we can have only one function where data would be the widget itself.
3212 * Wl_Cnp_Selection is global anyway so no need to put it as parameter.
3213 * dragwidget can be NULLified everytime.
3214 */
3200static void 3215static void
3201_wl_sel_obj_del(void *data, Evas *e EINA_UNUSED, Evas_Object *obj, void *event_info EINA_UNUSED) 3216_wl_sel_obj_del(void *data, Evas *e EINA_UNUSED, Evas_Object *obj, void *event_info EINA_UNUSED)
3202{ 3217{
@@ -3340,6 +3355,9 @@ _wl_elm_cnp_selection_get(Evas_Object *obj, Elm_Sel_Type selection, Elm_Sel_Form
3340 EVAS_CALLBACK_DEL, _wl_sel_obj_del2, 3355 EVAS_CALLBACK_DEL, _wl_sel_obj_del2,
3341 &wl_cnp_selection); 3356 &wl_cnp_selection);
3342 3357
3358 /* Why this check is not at the beginning?
3359 * More, I don't think you want to init stuff if the selection type is bad.
3360 */
3343 if ((selection == ELM_SEL_TYPE_CLIPBOARD) || 3361 if ((selection == ELM_SEL_TYPE_CLIPBOARD) ||
3344 (selection == ELM_SEL_TYPE_PRIMARY) || 3362 (selection == ELM_SEL_TYPE_PRIMARY) ||
3345 (selection == ELM_SEL_TYPE_SECONDARY)) 3363 (selection == ELM_SEL_TYPE_SECONDARY))
@@ -3392,6 +3410,7 @@ _wl_elm_cnp_selection_clear(Evas_Object *obj, Elm_Sel_Type selection EINA_UNUSED
3392 return EINA_TRUE; 3410 return EINA_TRUE;
3393} 3411}
3394 3412
3413/* If the function is common, it should be renamed. */
3395static Eina_Bool 3414static Eina_Bool
3396_wl_selection_send(void *data, int type EINA_UNUSED, void *event) 3415_wl_selection_send(void *data, int type EINA_UNUSED, void *event)
3397{ 3416{
@@ -3415,6 +3434,7 @@ _wl_selection_send(void *data, int type EINA_UNUSED, void *event)
3415 { 3434 {
3416 cnp_debug("Found a type: %s\n", atom->name); 3435 cnp_debug("Found a type: %s\n", atom->name);
3417 Dropable *drop; 3436 Dropable *drop;
3437 /* Why do we need it? Because it was in X11? */
3418 eo_do(sel->requestwidget, drop = eo_key_data_get("__elm_dropable")); 3438 eo_do(sel->requestwidget, drop = eo_key_data_get("__elm_dropable"));
3419 if (drop) 3439 if (drop)
3420 drop->last.type = atom->name; 3440 drop->last.type = atom->name;
@@ -3430,6 +3450,7 @@ _wl_selection_send(void *data, int type EINA_UNUSED, void *event)
3430 } 3450 }
3431 } 3451 }
3432 3452
3453 /* What if we don't find any atom? We don't send anything? */
3433 len_remained = len_ret; 3454 len_remained = len_ret;
3434 buf = data_ret; 3455 buf = data_ret;
3435 3456