On Thu, 31 Aug 2017 16:18:03 +0000 Mike Blumenkrantz <michael.blumenkra...@gmail.com> 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 <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 > -- ------------- 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
