Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimize an active window when its entry is clicked in the taskbar #234

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions uspace/app/taskbar/wndlist.c
Original file line number Diff line number Diff line change
Expand Up @@ -716,13 +716,27 @@ errno_t wndlist_repaint(wndlist_t *wndlist)
static void wndlist_button_clicked(ui_pbutton_t *pbutton, void *arg)
{
wndlist_entry_t *entry = (wndlist_entry_t *)arg;
wndmgt_window_info_t *winfo = NULL;
sysarg_t dev_id;
errno_t rc;
bool minimized = false;

/* ID of device that clicked the button */
dev_id = entry->wndlist->ev_idev_id;

(void) wndmgt_activate_window(entry->wndlist->wndmgt,
dev_id, entry->wnd_id);
rc = wndmgt_get_window_info(entry->wndlist->wndmgt,
entry->wnd_id, &winfo);
if (rc == EOK) {
minimized = (winfo->flags & wndf_minimized) == 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the logic is reversed w.r.t. the name of the variable: minimized will be true iff the window is not minimized

}

if (!minimized) {
(void) wndmgt_activate_window(entry->wndlist->wndmgt,
dev_id, entry->wnd_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation is incorrect. Did you run 'make ccheck'?

} else {
Copy link
Contributor

@jxsvoboda jxsvoboda Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now here's the big problem. If you click the button of a window that's not minimized and is not focused, instead of activating the window it will minimize it. Also note that with multiple seats, each window can have zero, one or more foci. A window should only be minimized if it is the focus of the same seat that clicked the window button. For example, you might add a WM operation to test if a particular window is focused by a particular seat (which is specified via input device ID).

(void) wndmgt_deactivate_window(entry->wndlist->wndmgt,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the operation should simply be called 'minimize', because that's what it does (this pertains to all other places too).

dev_id, entry->wnd_id);
}
}

/** @}
Expand Down
1 change: 1 addition & 0 deletions uspace/lib/wndmgt/include/ipc/wndmgt.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ typedef enum {
WNDMGT_GET_WINDOW_LIST,
WNDMGT_GET_WINDOW_INFO,
WNDMGT_ACTIVATE_WINDOW,
WNDMGT_DEACTIVATE_WINDOW,
WNDMGT_CLOSE_WINDOW,
WNDMGT_GET_EVENT,
} wndmgt_request_t;
Expand Down
1 change: 1 addition & 0 deletions uspace/lib/wndmgt/include/wndmgt.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ extern errno_t wndmgt_get_window_info(wndmgt_t *, sysarg_t,
wndmgt_window_info_t **);
extern void wndmgt_free_window_info(wndmgt_window_info_t *);
extern errno_t wndmgt_activate_window(wndmgt_t *, sysarg_t, sysarg_t);
extern errno_t wndmgt_deactivate_window(wndmgt_t *, sysarg_t, sysarg_t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this new operation unit tests should be added to /data/helenos/master/uspace/lib/wndmgt/test/wndmgt.c, similar to PCUT_TEST(activate_window_failure) and PCUT_TEST(activate_window_success).

extern errno_t wndmgt_close_window(wndmgt_t *, sysarg_t);

#endif
Expand Down
1 change: 1 addition & 0 deletions uspace/lib/wndmgt/include/wndmgt_srv.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct wndmgt_ops {
errno_t (*get_window_list)(void *, wndmgt_window_list_t **);
errno_t (*get_window_info)(void *, sysarg_t, wndmgt_window_info_t **);
errno_t (*activate_window)(void *, sysarg_t, sysarg_t);
errno_t (*deactivate_window)(void *, sysarg_t, sysarg_t);
errno_t (*close_window)(void *, sysarg_t);
errno_t (*get_event)(void *, wndmgt_ev_t *);
};
Expand Down
22 changes: 22 additions & 0 deletions uspace/lib/wndmgt/src/wndmgt.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,28 @@ errno_t wndmgt_activate_window(wndmgt_t *wndmgt, sysarg_t dev_id,
return rc;
}

/** Deactivate (minimize) window.
*
* @param wndmgt Window management session
* @param dev_id ID of input device belonging to seat whose
* focus is to be switched
* @param wnd_id Window ID
* @return EOK on success or an error code
*/
errno_t wndmgt_deactivate_window(wndmgt_t *wndmgt, sysarg_t dev_id,
sysarg_t wnd_id)
{
async_exch_t *exch;
errno_t rc;

exch = async_exchange_begin(wndmgt->sess);
rc = async_req_2_0(exch, WNDMGT_DEACTIVATE_WINDOW, dev_id,
wnd_id);

async_exchange_end(exch);
return rc;
}

/** Close window.
*
* @param wndmgt Window management
Expand Down
21 changes: 21 additions & 0 deletions uspace/lib/wndmgt/src/wndmgt_srv.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,24 @@ static void wndmgt_activate_window_srv(wndmgt_srv_t *srv, ipc_call_t *icall)
async_answer_0(icall, rc);
}

static void wndmgt_deactivate_window_srv(wndmgt_srv_t *srv, ipc_call_t *icall)
{
sysarg_t dev_id;
sysarg_t wnd_id;
errno_t rc;

dev_id = ipc_get_arg1(icall);
wnd_id = ipc_get_arg2(icall);

if (srv->ops->deactivate_window == NULL) {
async_answer_0(icall, ENOTSUP);
return;
}

rc = srv->ops->deactivate_window(srv->arg, dev_id, wnd_id);
async_answer_0(icall, rc);
}

static void wndmgt_close_window_srv(wndmgt_srv_t *srv, ipc_call_t *icall)
{
sysarg_t wnd_id;
Expand Down Expand Up @@ -308,6 +326,9 @@ void wndmgt_conn(ipc_call_t *icall, wndmgt_srv_t *srv)
case WNDMGT_ACTIVATE_WINDOW:
wndmgt_activate_window_srv(srv, &call);
break;
case WNDMGT_DEACTIVATE_WINDOW:
wndmgt_deactivate_window_srv(srv, &call);
break;
case WNDMGT_CLOSE_WINDOW:
wndmgt_close_window_srv(srv, &call);
break;
Expand Down
30 changes: 30 additions & 0 deletions uspace/srv/hid/display/wmops.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@
static errno_t dispwm_get_window_list(void *, wndmgt_window_list_t **);
static errno_t dispwm_get_window_info(void *, sysarg_t, wndmgt_window_info_t **);
static errno_t dispwm_activate_window(void *, sysarg_t, sysarg_t);
static errno_t dispwm_deactivate_window(void *, sysarg_t, sysarg_t);
static errno_t dispwm_close_window(void *, sysarg_t);
static errno_t dispwm_get_event(void *, wndmgt_ev_t *);

wndmgt_ops_t wndmgt_srv_ops = {
.get_window_list = dispwm_get_window_list,
.get_window_info = dispwm_get_window_info,
.activate_window = dispwm_activate_window,
.deactivate_window = dispwm_deactivate_window,
.close_window = dispwm_close_window,
.get_event = dispwm_get_event,
};
Expand Down Expand Up @@ -188,6 +190,34 @@ static errno_t dispwm_activate_window(void *arg, sysarg_t dev_id,
return EOK;
}

/** Deactivate (minimize) window.
*
* @param arg Argument (WM client)
* @param dev_id Input device ID
* @param wnd_id Window ID
* @return EOK on success or an error code
*/
static errno_t dispwm_deactivate_window(void *arg, sysarg_t dev_id,
sysarg_t wnd_id)
{
ds_wmclient_t *wmclient = (ds_wmclient_t *)arg;
ds_window_t *wnd;

log_msg(LOG_DEFAULT, LVL_DEBUG, "dispwm_deactivate_window()");

ds_display_lock(wmclient->display);
wnd = ds_display_find_window(wmclient->display, wnd_id);
if (wnd == NULL) {
ds_display_unlock(wmclient->display);
return ENOENT;
}

ds_window_minimize(wnd);

ds_display_unlock(wmclient->display);
return EOK;
}

/** Close window.
*
* @param arg Argument (WM client)
Expand Down