Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: e client focus - fix focus if moving focused window to new desk - long

2017-09-01 Thread Mike Blumenkrantz 
On Fri, Sep 1, 2017 at 6:29 AM Carsten Haitzler 
wrote:

> raster pushed a commit to branch master.
>
>
> http://git.enlightenment.org/core/enlightenment.git/commit/?id=9ae24a3a4a93827f6bce1c565a00926aa8ea1460
>
> commit 9ae24a3a4a93827f6bce1c565a00926aa8ea1460
> Author: Carsten Haitzler (Rasterman) 
> Date:   Fri Sep 1 19:29:03 2017 +0900
>
> e client focus - fix focus if moving focused window to new desk - long
>
> fix client focus the very very very long way vs
> 418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> ---
>  src/bin/e_actions.c  | 16 +--
>  src/bin/e_client.c   |  6 
>  src/bin/e_comp_x.c   | 25 ++--
>  src/bin/e_int_client_menu.c  | 10 ++-
>  src/modules/msgbus/msgbus_window.c   | 13 -
>  src/modules/pager/e_mod_main.c   | 55
> ++--
>  src/modules/pager/gadget/pager.c | 52
> +-
>  src/modules/pager_plain/e_mod_main.c | 54
> ++-
>  8 files changed, 157 insertions(+), 74 deletions(-)
>
>
>
I'm still not sold on the idea of warping the user's pointer after
finishing a pointer-based dnd operation, but at least this avoids warping
the pointer e.g., after applying window remembers, so thanks for this.

FWIW e_desk_last_focused_focus() already calls
e_client_focus_set_with_pointer() if appropriate and also raises the
focused client.
--
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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: e client focus - fix focus if moving focused window to new desk

2017-09-01 Thread The Rasterman 
On Thu, 31 Aug 2017 16:18:03 + Mike Blumenkrantz
 said:

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

that's a LOT of cases to check:

 2:16PM ~/C/e ⎇ master █▓░ git grep e_client_desk_set | wc -l
52

> On Thu, Aug 31, 2017 at 2:51 AM Carsten Haitzler 
> wrote:
> 
> > On Wed, 30 Aug 2017 14:27:09 + Mike Blumenkrantz
> >  said:
> >
> > > On Wed, Aug 30, 2017 at 3:14 AM Carsten Haitzler 
> > > wrote:
> > >
> > > > raster pushed a commit to branch master.
> > > >
> > > >
> > > >
> > http://git.enlightenment.org/core/enlightenment.git/commit/?id=418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> > > >
> > > > commit 418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> > > > Author: Carsten Haitzler (Rasterman) 
> > > > 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
> 


-- 
- Codito, ergo sum - "I

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: e client focus - fix focus if moving focused window to new desk

2017-08-31 Thread Mike Blumenkrantz 
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 
wrote:

> On Wed, 30 Aug 2017 14:27:09 + Mike Blumenkrantz
>  said:
>
> > On Wed, Aug 30, 2017 at 3:14 AM Carsten Haitzler 
> > wrote:
> >
> > > raster pushed a commit to branch master.
> > >
> > >
> > >
> http://git.enlightenment.org/core/enlightenment.git/commit/?id=418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> > >
> > > commit 418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> > > Author: Carsten Haitzler (Rasterman) 
> > > 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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: e client focus - fix focus if moving focused window to new desk

2017-08-31 Thread The Rasterman 
On Wed, 30 Aug 2017 14:27:09 + Mike Blumenkrantz
 said:

> On Wed, Aug 30, 2017 at 3:14 AM Carsten Haitzler 
> wrote:
> 
> > raster pushed a commit to branch master.
> >
> >
> > http://git.enlightenment.org/core/enlightenment.git/commit/?id=418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> >
> > commit 418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> > Author: Carsten Haitzler (Rasterman) 
> > 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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: e client focus - fix focus if moving focused window to new desk

2017-08-30 Thread Mike Blumenkrantz 
On Wed, Aug 30, 2017 at 3:14 AM Carsten Haitzler 
wrote:

> raster pushed a commit to branch master.
>
>
> http://git.enlightenment.org/core/enlightenment.git/commit/?id=418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
>
> commit 418319fc942a2c9204c8ec1d3d9e9bb1bbfb83fa
> Author: Carsten Haitzler (Rasterman) 
> 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.


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