Page MenuHomePhabricator

genlist: fix wrong returns in window tooltip set.
AbandonedPublic

Authored by SanghyeonLee on Feb 11 2019, 11:56 PM.

Details

Summary

tooltip_window_mode_set returns input disable value not a successibility of API actions.
when user set window_mode to false, it must return false regardless of item view existence.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9443
Build 7958: arc lint + arc unit
SanghyeonLee created this revision.Feb 11 2019, 11:56 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

SanghyeonLee requested review of this revision.Feb 11 2019, 11:56 PM
zmike requested changes to this revision.Feb 12 2019, 7:01 AM
zmike added a subscriber: zmike.

As the original author of both this functionality and the accompanying documentation, this patch is not correct. The intent was always to return false only in the case where a tooltip is currently active for the realized item and somehow switching this over to a windowed tooltip fails.

This revision now requires changes to proceed.Feb 12 2019, 7:01 AM
SanghyeonLee added a comment.EditedFeb 12 2019, 10:04 PM

but as you see the code,
in realize case,
it get the return from elm_widget_item_tooltip_window_mode_set which redirect elm_object_tooltip_window_mode_set,
and in elm_tooltip_window_mode_set,
it returns input disable.

if you not intend this code, then why the action is different in realize case?

the problem is,
we have a Tase Case which check negative case of API usage,
and it works when item is realized,
and it didn't works when item is unrealized.

if we don't store the window mode in the tooltip.free_size,
then the request need to be failed in unrealized item,
but we have this flag, and we restore the window mode by this data when item is ready,
so then why we don't give correct return value for this?

for me, it looks more natural to work this API regardless of item's realized status,
but if you think that is right way,
will fix the Test Case.
but please reconsider again. I think this fix make sense than current action.

SanghyeonLee abandoned this revision.Mon, Feb 25, 4:46 AM

it looks zmike disagreed and I respect orignal authors opinion.

zmike added a comment.Mon, Feb 25, 4:50 AM

Hm okay, you've convinced me.