Page MenuHomePhabricator

elementary: Check for valid focus manager before starting do loop
ClosedPublic

Authored by devilhorns on Jul 31 2018, 1:32 AM.

Details

Summary

Apparently there are cases where efl_ui_focus_object_focus_manager_get
can return NULL. In order to trap for this, we can simply modify the
do loop slightly and check for a valid manager before we actually
start looping

Test Case: elementary_test -to "Panel Scrollable" and click Toggle
button

ref T7030

Depends on D6703

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.
devilhorns created this revision.Jul 31 2018, 1:32 AM

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/

devilhorns requested review of this revision.Jul 31 2018, 1:32 AM
zmike edited projects, added Restricted Project; removed efl.
devilhorns updated this revision to Diff 15998.Jul 31 2018, 9:47 AM
devilhorns edited the summary of this revision. (Show Details)
This comment was removed by devilhorns.
bu5hm4n accepted this revision.Aug 2 2018, 4:16 AM
This revision is now accepted and ready to land.Aug 2 2018, 4:16 AM
bu5hm4n requested changes to this revision.Aug 2 2018, 4:18 AM

Noep this breaks setting focus to objects that are not visible yet.
the code from line 46 to 52 must be executed.

This revision now requires changes to proceed.Aug 2 2018, 4:18 AM
devilhorns added inline comments.Aug 2 2018, 10:52 AM
src/lib/elementary/efl_ui_focus_util.c
33

Would you prefer a check here to make sure that 'm' is valid ?

@bu5hm4n Would you prefer a check here to make sure that 'm' is valid ?

bu5hm4n added a comment.EditedAug 9 2018, 10:49 AM

Sorry, completly missed your reply.

Either that, or move the if(!m) block out of the while (after the loop body), that should also do it :)

devilhorns updated this revision to Diff 16274.Aug 13 2018, 4:33 AM

Moved if (!m) block to be outside the while loop

This revision was not accepted when it landed; it landed in state Needs Review.Aug 17 2018, 8:30 AM
This revision was automatically updated to reflect the committed changes.
devilhorns reopened this revision.Aug 17 2018, 8:35 AM
bu5hm4n requested changes to this revision.Aug 20 2018, 3:06 AM

Okay, deeply checked this. There seems to be a bug when focusing gengrid manually.

https://phab.enlightenment.org/P230

That function body works for me, does that also work for you ?

(Sorry for pinging arround on this one)

This revision now requires changes to proceed.Aug 20 2018, 3:06 AM

Yes, that block works

Okay, deeply checked this. There seems to be a bug when focusing gengrid manually.

https://phab.enlightenment.org/P230

That function body works for me, does that also work for you ?

(Sorry for pinging arround on this one)

Could you make patch about that?
I also tested and it works well.

Sure, I can change the patch to do that

devilhorns updated this revision to Diff 16702.Sep 6 2018, 6:21 AM

Update patch according to reviewed comments

@bu5hm4n Patch updated. Needs review again

bu5hm4n accepted this revision.Sep 6 2018, 9:20 AM
This revision is now accepted and ready to land.Sep 6 2018, 9:20 AM
Closed by commit rEFL84423a465ca2: elementary: Check for valid focus manager before starting do loop (authored by devilhorns, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhySep 6 2018, 9:29 AM
This revision was automatically updated to reflect the committed changes.