this fixes doing operations inside the constructor which require the use
of the theme object while avoiding a second call to theme_apply during
finalize if it previously succeeded during construction
Details
Details
- Reviewers
bu5hm4n
Diff Detail
Diff Detail
- Repository
- rEFL core/efl
- Branch
- master
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 11770
Comment Actions
I really dont know about this one. Should we really support this ? Basically every single call in the efl_add thing, that is not explicitly marked as constructor, is basically illegal.
Additionally, setting the class before constructor that this code path is hit does not really work nicely. Think about the inheritance like this: There is A B C, C inherits from B, B inherits from A. Now all of them are setting the class before super-constructor calls. What will happen ? A will have the klass set as last thing, before things are applied, this is probebly not what we want, (we want to have the setting after the super-constructor call, then the last element in the inherit chain gets the value set.
TLDR: i think it is fine that things are not allowed before finalize is called. only constructors should be called there.
Comment Actions
This doesn't change any existing mechanics, it just calls theme apply at the end of construction if the widget's class has already been set. As you can see, if the theme apply fails then it maintains the current behavior; I think this fallback will only be triggered for actual error cases, however.
For widgets which inherit from layout (all widgets inherit from layout), this means that since they've probably set their class already during their own constructor they will now immediately apply the layout theme. This fixes things like calling efl_text_set on widgets during extended construction, something that should definitely be allowed.
Comment Actions
It changes an existing mechanism, it starts to theme the object before all neccessary information are there. The object needs to be themed after the klass / style / element is set. Right now this patch only applies the theme after klass has been set. However, the style can be changed until finalize is done. So this breaks that behavior (i think). Further more, the style is meant to be set by the user. So no way to set this before finalizer is done. Additionally, we do not want to theme a object twice, as that is quite an heavy lift..
Comment Actions
Maybe we need to approach it a bit differently then. Things like packing an object into a box or setting hints during construction should always work (and they do without this patch), but also I think it's important that e.g., efl_text_set should always work on an object. So it may be the case that I need to improve edje internal caching somehow to allow "pre-setting" text and swallow parts or something...
Comment Actions
Improving edje to do so, might be doable to some extent. We already have the infrastructure to reload on theme file set. We could just have a case when the theme has not been set before and add things like it was a reload case. The main issue with this strategy is that we won't be able to answer question if a group exist and things like that. I don't know how much of this function that probe the loaded theme are used. If it is an important case to handle, we might have to thing of a different strategy. One I can think of is to add a new loading mechanism that will only do the loading when a function that need the file make a call and use the reload infrastructure in the rest of the case. This could help, just not sure what the typical scenario are.