Page MenuHomePhabricator

elm_image: keep backword compatibility for elm_image_file_set API when setting url file set twice
ClosedPublic

Authored by herb on Jun 15 2020, 11:22 PM.

Details

Summary

when trying to set file using url path twice, the second api call's return value is EINA_FALSE
since image obj has already has same file path. After applying Efl.File interface, the behavior is changed compared to before.
both of the return values should be EINA_TRUE.
@fix

ex)
Eina_Bool ret1, ret2;
ret1 = elm_image_file_set(image, "http://sameurl/image.jpg", NULL);
ret2 = elm_image_file_set(image, "http://sameurl/image.jpg", NULL);
ret1 and ret2 should be EINA_TURE

Test Plan
  1. call elm_image_file_set api with same url path
  2. see the return value

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
herb created this revision.Jun 15 2020, 11:22 PM
herb requested review of this revision.Jun 15 2020, 11:22 PM
herb updated this revision to Diff 30650.Jun 15 2020, 11:23 PM

update source codes

I cannot comment how it was with regards to backwards comp. but shoundn't this also check if the group is the same ?

Hermet requested changes to this revision.Jun 16 2020, 10:45 PM

Please keep code convention. We prefer simpler way.

src/lib/elementary/efl_ui_image.c
2458

if (is_file_path && file)

This revision now requires changes to proceed.Jun 16 2020, 10:45 PM
herb updated this revision to Diff 30669.Jun 17 2020, 2:37 AM

consider group

herb updated this revision to Diff 30670.Jun 17 2020, 2:38 AM

modify condition

herb updated this revision to Diff 30671.Jun 17 2020, 2:45 AM

update condition

jsuya added a comment.Jun 17 2020, 2:47 AM

In my opinion, if Efl.Ui.Image needs to do the same, I think I can handle this on line 939. Is it only necessary to support legacy?

jsuya added inline comments.Jun 17 2020, 2:52 AM
src/lib/elementary/efl_ui_image.c
2465

How about this? if ( (!cur_group && !group) || (cur_group && group && !strcmp(cur_group, group)) )

2474

Please check the indentation.

jsuya requested changes to this revision.Jun 17 2020, 2:53 AM

Please check my comment.

This revision now requires changes to proceed.Jun 17 2020, 2:53 AM
herb updated this revision to Diff 30674.Jun 17 2020, 2:58 AM

update condition

herb added a comment.Jun 17 2020, 3:01 AM

In my opinion, if Efl.Ui.Image needs to do the same, I think I can handle this on line 939. Is it only necessary to support legacy?

This patch is for legacy backword compatibility issue, I think there is no responsibility for efl_ui_image behavior.

herb updated this revision to Diff 30676.Jun 17 2020, 3:23 AM

clean up the codes

herb updated this revision to Diff 30679.Jun 17 2020, 5:02 AM

update codes

Hermet accepted this revision.Jun 17 2020, 5:03 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 17 2020, 5:04 AM
This revision was automatically updated to reflect the committed changes.