On Wed, 30 Aug 2017 14:27:09 +0000 Mike Blumenkrantz
<michael.blumenkra...@gmail.com> said:

> On Wed, Aug 30, 2017 at 3:14 AM Carsten Haitzler <ras...@rasterman.com>
> wrote:
> 
> > raster pushed a commit to branch master.
> >
> >
> > http://git.enlightenment.org/core/enlightenment.git/commit/?id=418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> >
> > commit 418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> > Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com>
> > Date:   Wed Aug 30 16:13:50 2017 +0900
> >
> >     e client focus - fix focus if moving focused window to new desk
> >
> >     if the window being moved to a new desktop is focused, then ensure
> >     after the move to restore focus to the last focused in the focus stack
> >     for this desk to something stays focused.
> >
> >     @fix
> >
> 
> What scenario is this attempting to resolve? There are cases where moving a
> focused client off the active desk should not result in another client
> being focused.

when a user actively moves a focused window off the current desk to another they
end up with zero windows focused (when other windows still are on the desktop).
complaint directly from user. move by pager. by menu. by action as some of the
paths at least.

> > ---
> >  src/bin/e_client.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/bin/e_client.c b/src/bin/e_client.c
> > index d3dc6bdef..414220422 100644
> > --- a/src/bin/e_client.c
> > +++ b/src/bin/e_client.c
> > @@ -2805,6 +2805,7 @@ e_client_desk_set(E_Client *ec, E_Desk *desk)
> >  {
> >     E_Event_Client_Desk_Set *ev;
> >     E_Desk *old_desk;
> > +   Eina_Bool was_focused = ec->focused;
> >
> 
> This is a forward null dereference and will create a new coverity issue.
> 
> 
> >
> >     E_OBJECT_CHECK(ec);
> >     E_OBJECT_TYPE_CHECK(ec, E_CLIENT_TYPE);
> > @@ -2872,6 +2873,11 @@ e_client_desk_set(E_Client *ec, E_Desk *desk)
> >               e_client_res_change_geometry_restore(ec);
> >               ec->pre_res_change.valid = 0;
> >            }
> > +        if (was_focused)
> > +          {
> > +             E_Client *ec_focus = e_desk_last_focused_focus(old_desk);
> >
> 
> This function already performs a focus set where possible.
> 
> 
> > +             if (ec_focus) e_client_focus_set_with_pointer(ec_focus);
> >
> 
> Manually forcing focus after the above function guarantees more focus bugs,
> see 81a797a0fa1dbe3c979c97351b714e4b5a8024ee.
> 
> 
> > +          }
> >       }
> >
> >     if (ec->stack.prev || ec->stack.next)
> >
> > --
> >
> >
> >
> Overall, it seems to me like this is not an ideal change and that focus
> setting should be performed independently of this function in order to
> avoid creating more bugs.
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    ras...@rasterman.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to