Not all user complaints are items that we should rush to address by adding
null checks.

This change, for example, has significant downsides:
* causes new/unwanted pointer warping after pointer-initiated operations
* causes new/unwanted pointer warping after menu uses
* causes new/unwanted pointer warping after applying a window remember

In all cases except the case where the user is using click focus and does
one of (move to desk from menu | pager drag | specific move client to desk
action) this is broken. It seems much better to just handle those specific
cases.

On Thu, Aug 31, 2017 at 2:51 AM Carsten Haitzler <ras...@rasterman.com>
wrote:

> 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