bu5hm4n pushed a commit to branch master.
http://git.enlightenment.org/core/efl.git/commit/?id=c51f35d42a148b34a9cf74714ecc16609984eecb
commit c51f35d42a148b34a9cf74714ecc16609984eecb
Author: Marcel Hollerbach <marcel-hollerb...@t-online.de>
Date: Sat Sep 2 19:08:18 2017 +0200
elm_widget: move the complete regsiter/unregister logic
We had here a little problem, state focus_state_eval function handled
the unregisteration and consideration of the focus flags and then only
called a helper function (which was a widget function), that then did
the registeration in logical or regular mode.
Elm scroller for example took that function overwrote it and did onyl
permit logical registrations. Then again a evaluation of the focus state
and flags took place, and the function considered elm_scroller should be
registered as regular object, but found it to be logical. This lead to
the problem that we permantently unregistered Elm.Scroller and
registered it again as logical just to unregister it again. This was on
the one side a performance downside. But also a bug since all items from
within the Elm_Scrollers sub manager are getting reparent onto the
parent, which means not the root of the scroller (the scroller itself)
is the logical entrypoint to the widget but rather this reparented
widget, which led to unexpected focus warps like described in T5923.
tldr: this fixes T5923
---
src/lib/elementary/elc_fileselector.c | 4 +-
src/lib/elementary/elm_box.c | 7 ++-
src/lib/elementary/elm_box.eo | 2 +-
src/lib/elementary/elm_fileselector.eo | 2 +-
src/lib/elementary/elm_scroller.c | 6 +-
src/lib/elementary/elm_scroller.eo | 2 +-
src/lib/elementary/elm_table.c | 7 ++-
src/lib/elementary/elm_table.eo | 2 +-
src/lib/elementary/elm_toolbar.c | 6 +-
src/lib/elementary/elm_toolbar.eo | 2 +-
src/lib/elementary/elm_widget.c | 107 +++++++++++++++++++++------------
src/lib/elementary/elm_widget.eo | 27 ++++++---
src/lib/elementary/elm_widget.h | 6 +-
13 files changed, 112 insertions(+), 68 deletions(-)
diff --git a/src/lib/elementary/elc_fileselector.c
b/src/lib/elementary/elc_fileselector.c
index a3536c4b33..3308aef974 100644
--- a/src/lib/elementary/elc_fileselector.c
+++ b/src/lib/elementary/elc_fileselector.c
@@ -3104,9 +3104,9 @@
_elm_fileselector_elm_widget_focus_direction_manager_is(Eo *obj EINA_UNUSED, Elm
}
EOLIAN static Eina_Bool
-_elm_fileselector_elm_widget_focus_register(Eo *obj, Elm_Fileselector_Data
*pd, Efl_Ui_Focus_Manager *manager, Efl_Ui_Focus_Object *logical, Eina_Bool
*logical_flag)
+_elm_fileselector_elm_widget_focus_state_apply(Eo *obj, Elm_Fileselector_Data
*pd, Elm_Widget_Focus_State current_state, Elm_Widget_Focus_State
*configured_state, Elm_Widget *redirect)
{
- Eina_Bool ret = elm_obj_widget_focus_register(efl_super(obj, MY_CLASS),
manager, logical, logical_flag);
+ Eina_Bool ret = elm_obj_widget_focus_state_apply(efl_super(obj, MY_CLASS),
current_state, configured_state, redirect);
_focus_chain_update(obj, pd);
diff --git a/src/lib/elementary/elm_box.c b/src/lib/elementary/elm_box.c
index 04420be35f..224075d310 100644
--- a/src/lib/elementary/elm_box.c
+++ b/src/lib/elementary/elm_box.c
@@ -720,12 +720,13 @@ _elm_box_class_constructor(Efl_Class *klass)
evas_smart_legacy_type_register(MY_CLASS_NAME_LEGACY, klass);
}
EOLIAN Eina_Bool
-_elm_box_elm_widget_focus_register(Eo *obj, Elm_Box_Data *pd,
Efl_Ui_Focus_Manager *manager, Efl_Ui_Focus_Object *logical, Eina_Bool
*logical_flag)
+_elm_box_elm_widget_focus_state_apply(Eo *obj, Elm_Box_Data *pd,
Elm_Widget_Focus_State current_state, Elm_Widget_Focus_State *configured_state,
Elm_Widget *redirect)
{
- Eina_Bool result = elm_obj_widget_focus_register(efl_super(obj, MY_CLASS),
manager, logical, logical_flag);
+ Eina_Bool result = elm_obj_widget_focus_state_apply(efl_super(obj,
MY_CLASS), current_state, configured_state, redirect);
//later registering children are automatically set into the order of the
internal table
- _focus_order_flush(obj, pd);
+ if (configured_state->manager)
+ _focus_order_flush(obj, pd);
return result;
}
diff --git a/src/lib/elementary/elm_box.eo b/src/lib/elementary/elm_box.eo
index efdb2e3838..0269d84945 100644
--- a/src/lib/elementary/elm_box.eo
+++ b/src/lib/elementary/elm_box.eo
@@ -255,7 +255,7 @@ class Elm.Box (Elm.Widget)
Elm.Widget.focus_next;
Elm.Widget.theme_apply;
Elm.Widget.widget_sub_object_del;
- Elm.Widget.focus_register;
+ Elm.Widget.focus_state_apply;
}
events {
child,added; [[Called when child was added]]
diff --git a/src/lib/elementary/elm_fileselector.eo
b/src/lib/elementary/elm_fileselector.eo
index 3ac04a40db..ee33eb5ee5 100644
--- a/src/lib/elementary/elm_fileselector.eo
+++ b/src/lib/elementary/elm_fileselector.eo
@@ -41,7 +41,7 @@ class Elm.Fileselector (Efl.Ui.Layout,
Elm.Interface.Fileselector,
Elm.Widget.widget_event;
Elm.Widget.theme_apply;
Elm.Widget.focus_next_manager_is;
- Elm.Widget.focus_register;
+ Elm.Widget.focus_state_apply;
Elm.Interface.Fileselector.selected_models { get; }
Elm.Interface.Fileselector.selected_model_get;
Elm.Interface.Fileselector.selected_model_set;
diff --git a/src/lib/elementary/elm_scroller.c
b/src/lib/elementary/elm_scroller.c
index f764988c91..2c06b1aefa 100644
--- a/src/lib/elementary/elm_scroller.c
+++ b/src/lib/elementary/elm_scroller.c
@@ -1446,11 +1446,11 @@ _elm_scroller_class_constructor(Efl_Class *klass)
}
EOLIAN static Eina_Bool
-_elm_scroller_elm_widget_focus_register(Eo *obj, Elm_Scroller_Data *pd
EINA_UNUSED, Efl_Ui_Focus_Manager *manager, Efl_Ui_Focus_Object *logical,
Eina_Bool *logical_flag)
+_elm_scroller_elm_widget_focus_state_apply(Eo *obj, Elm_Scroller_Data *pd
EINA_UNUSED, Elm_Widget_Focus_State current_state, Elm_Widget_Focus_State
*configured_state, Elm_Widget *redirect EINA_UNUSED)
{
//undepended from logical or not we always reigster as full with ourself as
redirect
- *logical_flag = EINA_TRUE;
- return efl_ui_focus_manager_calc_register_logical(manager, obj, logical,
obj);
+ configured_state->logical = EINA_TRUE;
+ return elm_obj_widget_focus_state_apply(efl_super(obj, MY_CLASS),
current_state, configured_state, obj);
}
diff --git a/src/lib/elementary/elm_scroller.eo
b/src/lib/elementary/elm_scroller.eo
index 48f3f74754..94a1159916 100644
--- a/src/lib/elementary/elm_scroller.eo
+++ b/src/lib/elementary/elm_scroller.eo
@@ -60,7 +60,7 @@ class Elm.Scroller (Efl.Ui.Layout, Elm.Interface_Scrollable,
Elm.Interface_Scrollable.single_direction { get; set; }
Elm.Interface.Atspi_Widget_Action.elm_actions { get; }
Efl.Part.part;
- Elm.Widget.focus_register;
+ Elm.Widget.focus_state_apply;
}
events {
scroll,page,changed; [[Called when scroll page changed]]
diff --git a/src/lib/elementary/elm_table.c b/src/lib/elementary/elm_table.c
index d90bdc7d8d..dd40369ddb 100644
--- a/src/lib/elementary/elm_table.c
+++ b/src/lib/elementary/elm_table.c
@@ -419,12 +419,13 @@ _elm_table_efl_canvas_group_group_calculate(Eo *obj, void
*pd EINA_UNUSED)
}
EOLIAN Eina_Bool
-_elm_table_elm_widget_focus_register(Eo *obj, void *pd EINA_UNUSED,
Efl_Ui_Focus_Manager *manager, Efl_Ui_Focus_Object *logical, Eina_Bool
*logical_flag)
+_elm_table_elm_widget_focus_state_apply(Eo *obj, void *pd EINA_UNUSED,
Elm_Widget_Focus_State current_state, Elm_Widget_Focus_State *configured_state,
Elm_Widget *redirect)
{
- Eina_Bool result = elm_obj_widget_focus_register(efl_super(obj, MY_CLASS),
manager, logical, logical_flag);
+ Eina_Bool result = elm_obj_widget_focus_state_apply(efl_super(obj,
MY_CLASS), current_state, configured_state, redirect);
//later registering children are automatically set into the order of the
internal table
- _focus_order_flush(obj);
+ if (configured_state->manager)
+ _focus_order_flush(obj);
return result;
}
diff --git a/src/lib/elementary/elm_table.eo b/src/lib/elementary/elm_table.eo
index 9354068630..0c0536b18b 100644
--- a/src/lib/elementary/elm_table.eo
+++ b/src/lib/elementary/elm_table.eo
@@ -127,6 +127,6 @@ class Elm.Table (Elm.Widget)
Elm.Widget.focus_direction_manager_is;
Elm.Widget.theme_apply;
Elm.Widget.widget_sub_object_del;
- Elm.Widget.focus_register;
+ Elm.Widget.focus_state_apply;
}
}
diff --git a/src/lib/elementary/elm_toolbar.c b/src/lib/elementary/elm_toolbar.c
index 066d675efd..b947e86116 100644
--- a/src/lib/elementary/elm_toolbar.c
+++ b/src/lib/elementary/elm_toolbar.c
@@ -3074,10 +3074,10 @@ elm_toolbar_add(Evas_Object *parent)
}
EOLIAN static Eina_Bool
-_elm_toolbar_elm_widget_focus_register(Eo *obj, Elm_Toolbar_Data *pd
EINA_UNUSED, Efl_Ui_Focus_Manager *manager, Efl_Ui_Focus_Object *logical,
Eina_Bool *logical_flag)
+_elm_toolbar_elm_widget_focus_state_apply(Eo *obj, Elm_Toolbar_Data *pd
EINA_UNUSED, Elm_Widget_Focus_State current_state, Elm_Widget_Focus_State
*configured_state, Elm_Widget *redirect)
{
- *logical_flag = EINA_TRUE;
- return efl_ui_focus_manager_calc_register_logical(manager, obj, logical,
NULL);
+ configured_state->logical = EINA_TRUE;
+ return elm_obj_widget_focus_state_apply(efl_super(obj, MY_CLASS),
current_state, configured_state, redirect);
}
EOLIAN static Eo *
diff --git a/src/lib/elementary/elm_toolbar.eo
b/src/lib/elementary/elm_toolbar.eo
index c9ddc53a12..8864fabbc7 100644
--- a/src/lib/elementary/elm_toolbar.eo
+++ b/src/lib/elementary/elm_toolbar.eo
@@ -330,7 +330,7 @@ class Elm.Toolbar (Elm.Widget, Elm.Interface_Scrollable,
Efl.Ui.Direction,
Elm.Widget.focus_highlight_geometry { get; }
Elm.Widget.focused_item { get; }
Efl.Ui.Direction.direction { get; set; [[Only supports $vertical and
$horizontal. Default is $horizontal.]] }
- Elm.Widget.focus_register;
+ Elm.Widget.focus_state_apply;
Elm.Interface.Atspi_Widget_Action.elm_actions { get; }
Elm.Interface.Atspi_Accessible.children { get; }
Elm.Interface.Atspi_Accessible.state_set { get; }
diff --git a/src/lib/elementary/elm_widget.c b/src/lib/elementary/elm_widget.c
index 92e7e21a09..523a34eb2e 100644
--- a/src/lib/elementary/elm_widget.c
+++ b/src/lib/elementary/elm_widget.c
@@ -332,24 +332,63 @@ _focus_manager_eval(Eo *obj, Elm_Widget_Smart_Data *pd)
}
EOLIAN static Eina_Bool
-_elm_widget_focus_register(Eo *obj, Elm_Widget_Smart_Data *pd EINA_UNUSED,
- Efl_Ui_Focus_Manager *manager,
- Efl_Ui_Focus_Object *logical, Eina_Bool *logical_flag)
+_elm_widget_focus_state_apply(Eo *obj, Elm_Widget_Smart_Data *pd EINA_UNUSED,
Elm_Widget_Focus_State current_state, Elm_Widget_Focus_State *configured_state,
Elm_Widget *redirect)
{
+ Eina_Bool registered = EINA_TRUE;
- if (!*logical_flag)
- return efl_ui_focus_manager_calc_register(manager, obj, logical, NULL);
- else
- return efl_ui_focus_manager_calc_register_logical(manager, obj, logical,
NULL);
-}
+ //shortcut for having the same configurations
+ if (current_state.manager == configured_state->manager &&
!current_state.manager)
+ return !!current_state.manager;
+
+ if (configured_state->logical == current_state.logical &&
+ configured_state->manager == current_state.manager &&
+ configured_state->parent == current_state.parent)
+ return !!current_state.manager;
+
+ //this thing doesnt want to be registered, but it is ...
+ if (!configured_state->manager && current_state.manager)
+ {
+ efl_ui_focus_manager_calc_unregister(current_state.manager, obj);
+ return EINA_FALSE;
+ }
+ //by that point we have always a configured manager
+
+ if (!current_state.manager) registered = EINA_FALSE;
+
+ if (//check if we have changed the manager
+ (current_state.manager != configured_state->manager) ||
+ //check if we are already registered but in a different state
+ (current_state.logical != configured_state->logical))
+ {
+ //we need to unregister here
+ efl_ui_focus_manager_calc_unregister(current_state.manager, obj);
+ registered = EINA_FALSE;
+ }
+ if (!registered)
+ {
+ if (configured_state->logical)
+ return
efl_ui_focus_manager_calc_register_logical(configured_state->manager, obj,
configured_state->parent, redirect);
+ else
+ return efl_ui_focus_manager_calc_register(configured_state->manager,
obj, configured_state->parent, redirect);
+ }
+ ERR("Uncaught focus state consider this as unregistered (%d) \n (%p,%p,%d)
\n (%p,%p,%d) ", registered,
+ configured_state->manager, configured_state->parent,
configured_state->logical,
+ current_state.manager, current_state.parent, current_state.logical
+ );
+ abort();
+ return EINA_FALSE;
+}
static void
_focus_state_eval(Eo *obj, Elm_Widget_Smart_Data *pd)
{
Eina_Bool should = EINA_FALSE;
Eina_Bool want_full = EINA_FALSE;
- Efl_Ui_Focus_Manager *manager = pd->manager.manager;
+ Elm_Widget_Focus_State configuration;
+
+ //this would mean we are registering again the root, we dont want that
+ if (pd->manager.manager == obj) return;
//there are two reasons to be registered, the child count is bigger than 0,
or the widget is flagged to be able to handle focus
if (pd->can_focus)
@@ -379,44 +418,38 @@ _focus_state_eval(Eo *obj, Elm_Widget_Smart_Data *pd)
if (_tree_disabled(obj))
should = EINA_FALSE;
+
+ if (!evas_object_visible_get(obj))
+ should = EINA_FALSE;
}
- if ( //check if we have changed the manager
- (pd->focus.manager != manager && should) ||
- //check if we are already registered but in a different state
- (pd->focus.manager && should && want_full == pd->focus.logical)
- )
+ if (should)
{
- efl_ui_focus_manager_calc_unregister(pd->focus.manager, obj);
- pd->focus.manager = NULL;
- pd->focus.parent = NULL;
+ configuration.parent = pd->logical.parent;
+ configuration.manager = pd->manager.manager;
+ configuration.logical = !want_full;
}
-
- //now register in the manager
- if (should && !pd->focus.manager)
+ else
{
- if (manager && manager != obj)
- {
- if (!pd->logical.parent) return;
-
- pd->focus.manager = manager;
- pd->focus.logical = !want_full;
- pd->focus.parent = pd->logical.parent;
-
- if (!elm_obj_widget_focus_register(obj, pd->focus.manager,
- pd->focus.parent, &pd->focus.logical))
- {
- pd->focus.manager = NULL;
- pd->focus.parent = NULL;
- }
- }
+ configuration.parent = NULL;
+ configuration.manager = NULL;
+ configuration.logical = EINA_FALSE;
}
- else if (!should && pd->focus.manager)
+
+ if (!elm_obj_widget_focus_state_apply(obj, pd->focus, &configuration, NULL))
{
- efl_ui_focus_manager_calc_unregister(pd->focus.manager, obj);
+ //things went wrong or this thing is unregistered. Purge the current
configuration.
pd->focus.manager = NULL;
pd->focus.parent = NULL;
+ pd->focus.logical = EINA_FALSE;
}
+ else
+ {
+ pd->focus.parent = configuration.parent;
+ pd->focus.manager = configuration.manager;
+ pd->focus.logical = configuration.logical;
+ }
+
}
static Efl_Ui_Focus_Object*
diff --git a/src/lib/elementary/elm_widget.eo b/src/lib/elementary/elm_widget.eo
index ebeea3dc14..89d622f159 100644
--- a/src/lib/elementary/elm_widget.eo
+++ b/src/lib/elementary/elm_widget.eo
@@ -10,6 +10,13 @@ function Efl.Ui.Scrollable_On_Show_Region {
struct @extern Elm.Theme; [[Elementary theme]]
+struct Elm.Widget.Focus_State {
+ [[All relevant fields needed for the current state of focus registeration]]
+ manager : Efl.Ui.Focus.Manager; [[The manager where the widget is
registered in]]
+ parent : Efl.Ui.Focus.User; [[The parent the widget is using as logical
parent]]
+ logical : bool; [[$true if this is registered as logical currently]]
+}
+
abstract Elm.Widget (Efl.Canvas.Group, Elm.Interface.Atspi_Accessible,
Elm.Interface.Atspi_Component, Efl.Ui.Focus.User,
Efl.Ui.Focus.Object, Efl.Ui.Base, Efl.Ui.Cursor)
@@ -794,15 +801,21 @@ abstract Elm.Widget (Efl.Canvas.Group,
Elm.Interface.Atspi_Accessible,
}
/* Focus Manager API */
- focus_register @protected {
- [[Register focus with the given focus manager.]]
+ focus_state_apply @protected {
+ [[Register focus with the given configuration.
+
+ The implementation can feel free to change the logical flag as it
wants, but other than that it should strictly keep the configuration.
+
+ The implementation in elm.widget updates the current state into
what is passed as configured state, respecting manager changes, registeration
and unregistration based on if it should be registered or unregistered.
+
+ A manager field that is $null means that the widget should not or
was not registered.
+ ]]
params {
- @in manager: Efl.Ui.Focus.Manager; [[The focus manager to register
with.]]
- @in logical: Efl.Ui.Focus.Object; [[The logical parent to use.]]
- /* FIXME: The below doc needs fixing! */
- @inout logical_flag: bool; [[reference to the flag indicating if
the should be logical or not change this flag to the value you have it
registered]]
+ @in current_state : Elm.Widget.Focus_State; [[The focus manager to
register with.]]
+ @inout configured_state : Elm.Widget.Focus_State; [[The evalulated
Focus state that should be used]]
+ @in redirect : Elm.Widget; [[A redirect that will be set by the
elm.widget implementation]]
}
- return: bool; [[return $true or $false if the registration was
successfull or not]]
+ return: bool; [[return $true or $false if the widget is registered or
false if it is not]]
}
focus_manager_factory @protected {
[[If the widget needs a focus manager, this function will be called.
diff --git a/src/lib/elementary/elm_widget.h b/src/lib/elementary/elm_widget.h
index 3bd9b02521..8f3eae2ee0 100644
--- a/src/lib/elementary/elm_widget.h
+++ b/src/lib/elementary/elm_widget.h
@@ -430,11 +430,7 @@ typedef struct _Elm_Widget_Smart_Data
Elm_Focus_Move_Policy focus_move_policy;
Elm_Focus_Region_Show_Mode focus_region_show_mode;
- struct {
- Efl_Ui_Focus_Manager *manager; //manager which is currently regsitered in
- Efl_Ui_Focus_Object *parent; //the parent where it is currently registered
- Eina_Bool logical;
- } focus;
+ Elm_Widget_Focus_State focus;
struct {
int child_count;
Efl_Ui_Focus_Object *parent;
--
