Page MenuHomePhabricator

elm_main: fix backward compatibility of elm_object_focus_region_show_mode_set/get
AbandonedPublic

Authored by YOhoho on Mon, Mar 11, 5:03 AM.

Details

Summary

This is public API. even if you have used for only gengrid, the pulic API user
can use it for another widget. we have to ensure same result as before.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10165
Build 8166: arc lint + arc unit
YOhoho created this revision.Mon, Mar 11, 5:03 AM
YOhoho requested review of this revision.Mon, Mar 11, 5:03 AM
segfaultxavi edited reviewers, added: cedric, zmike, bu5hm4n; removed: segfaultxavi.Mon, Mar 11, 5:15 AM

Sorry, not my area of expertise.

I can see you intention, but this hides a bug, the API is there to adjust what interest_region_get is returning. And when you set it to item, only gengrid will respect that. So all in all you now can set that, verify that it is really set, and get the coordinates, just to realize that you did not get the item but rather the widget.
No other widget than gengrid does implement this, so whenever you used that outside gengrid, then you are asking for trouble, you did something in your code which is not working ...
If there is another widget, then this should probebly also be added to this, but i pretty much don't see a single usecase where this helps ... Additionally, i would actaully prefer focus_region_show_mode not beeing part of the Efl_Ui_Widget private data. As this is not used there...

@YOhoho do you have a usecase where this definitly needs to return a wrong value ?

YOhoho abandoned this revision.Sun, Mar 17, 7:17 AM

There is a unit test failure reported by tizen unit test.

Evas_Object *list = elm_list_add(p);
elm_object_focus_region_show_mode_set(list, ELM_FOCUS_REGION_SHOW_ITEM);
if (elm_object_focus_region_show_mode_get(list) != ELM_FOCUS_REGION_SHOW_ITEM)
  test fail

As you said, only gengrid implements that property. so i think it is invalid unit test. list widget was replaced with gengrid widget. never mind about this patch.

By the way, we can add documentation this API only work for gengrid.

Sure! I added a revision for it :)