Page MenuHomePhabricator

ui/text: Postpone scrollable property in efl_add
Needs RevisionPublic

Authored by jpeg on Nov 27 2018, 2:07 AM.

Details

Summary

Setting the scrollable property too early can leave the widget
completely unusable, it swallows and re-swallows the entry_edje internal
object.

Test Plan
elementary_test -to "gfx filters".

The bottom half of the pane was desperately empty.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 8230
Build 7514: arc lint + arc unit
jpeg created this revision.Nov 27 2018, 2:07 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/

jpeg requested review of this revision.Nov 27 2018, 2:07 AM
segfaultxavi accepted this revision.EditedNov 28 2018, 1:16 AM

Fixes the gfx filters elementary_test as advertised. This also fixes the Texteditor example app without having to modify the sample code (T7468).
However, I am not landing this as I would like somebody with more experience in Eo internals to review it.

This revision is now accepted and ready to land.Nov 28 2018, 1:16 AM
herdsman added a subscriber: herdsman.EditedNov 28 2018, 2:18 AM

Please give me today to just see that everything is in order.
This is a proper fix to T7468, and I intended to fix it similarly.

This is related to T7477, in which we discussed our over-use of setting properties during the construction of the object and the problems that may rise (as in this example, the object isn't finalized).
Since the theme is only applied when the widget is finalized (and only ones in its lifetime), such problems may arise.
Now, we could start doing efl_finalized_get checks with all properties (unlikely for methods, but users can actually call methods as well, in C), but we probably want to enforce that internally.

Of course, a crash needs to be fixed, and the patch is good.

The only "change" I am interested in is to add this property to the constructors block. I am not sure if it applies to optional parameters.
In any case, I would like to read your opinion.

herdsman requested changes to this revision.EditedNov 28 2018, 2:18 AM

Changing status so we can further discuss this before landing.

This revision now requires changes to proceed.Nov 28 2018, 2:18 AM