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
This commit is contained in:
Marcel Hollerbach 2017-09-02 19:08:18 +02:00
parent 3af81932b0
commit c51f35d42a
13 changed files with 114 additions and 70 deletions

View File

@ -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);

View File

@ -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;
}

View File

@ -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]]

View File

@ -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;

View File

@ -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);
}

View File

@ -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]]

View File

@ -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;
}

View File

@ -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;
}
}

View File

@ -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 *

View File

@ -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; }

View File

@ -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);
configuration.parent = pd->logical.parent;
configuration.manager = pd->manager.manager;
configuration.logical = !want_full;
}
else
{
configuration.parent = NULL;
configuration.manager = NULL;
configuration.logical = EINA_FALSE;
}
if (!elm_obj_widget_focus_state_apply(obj, pd->focus, &configuration, NULL))
{
//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;
}
//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;
}
}
}
else if (!should && pd->focus.manager)
{
efl_ui_focus_manager_calc_unregister(pd->focus.manager, obj);
pd->focus.manager = NULL;
pd->focus.parent = NULL;
pd->focus.parent = configuration.parent;
pd->focus.manager = configuration.manager;
pd->focus.logical = configuration.logical;
}
}
static Efl_Ui_Focus_Object*

View File

@ -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.

View File

@ -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;