Skip Menu |
 

Subject: NIM GUI: views jump around on the screen
NIM from either MIT KfW Beta 2 or from secure-
endpoints.com/binaries/mit-kfw-3-2-0/kfw-3-2-0.msi.

Vista or XP.


The standard and advanced views are displayed in different places on
the desktop. Their upper left corners should be in the same place.
[kpkoch - Mon Jul 30 10:45:31 2007]:

Show quoted text
> The standard and advanced views are displayed in different places on
> the desktop. Their upper left corners should be in the same place.

This is a feature not a bug. The basic and advanced views are shown in
different locations because they are different sizes. If you line then
up on the top-left corner then when the user switches views if the
advanced window does not fit on the display, the user must then drag it
up and to the left. When the user switches back to the basic view, the
user may have to drag it down and to the right in order to restore it to
the user's favorite viewing location.

For me, I keep NIM's basic view pinned to the bottom right corner of the
display just above the tray. For advanced view I want the window
centered on the display so I can fully expand all of the acquired
credentials and show all of my selected columns.

Jeffrey Altman
That sounds reasonable. The problem then becomes that when a view is
dragged to a new location, it doesn't stay there unless a pushpin is
toggled twice. That is, DRAG, F7, F7 shows the view in its pre-drag
location. DRAG, PUSH, PUSH is required for F7 F7 to leave it where I
put it.

It doesn't make sense for a pushpin on an identity to affect the
view. A pushpin on an identity should be related to the identity.

Why not always remember where the view is and always put it back where
it was?
Date: Thu, 02 Aug 2007 13:37:22 -0400
From: Jeffrey Altman <jaltman@secure-endpoints.com>
To: kfw-bugs@mit.edu
Subject: Re: [krbdev.mit.edu #5613] NIM GUI: views jump around on the screen
RT-Send-Cc:
Kevin Koch via RT wrote:
Show quoted text
> That sounds reasonable. The problem then becomes that when a view is
> dragged to a new location, it doesn't stay there unless a pushpin is
> toggled twice. That is, DRAG, F7, F7 shows the view in its pre-drag
> location. DRAG, PUSH, PUSH is required for F7 F7 to leave it where I
> put it.

The bug is that toggling View->Advanced does not save the current
location. Fix that. Push-pins have nothing to do with the location of
the Application Window regardless of View.
Download smime.p7s
application/x-pkcs7-signature 3.2KiB

Message body not shown because it is not plain text.

Download (untitled) / with headers
text/plain 4.4KiB
Show quoted text
> The bug is that toggling View->Advanced does not save the current
> location. Fix that. Push-pins have nothing to do with the location
> of the Application Window regardless of View.

Actually, the bug was that the size & position were being remembered
at the wrong time.

The good news is that the patch below fixes the problem. The bad news
is that it doesn't offer any insight into the display inconsistency
bugs.

Patch:

Index: mainwnd.c
===================================================================
--- mainwnd.c (revision 19745)
+++ mainwnd.c (working copy)
@@ -205,6 +205,45 @@
}
}

+void main_wnd_save_sizepos(HWND hwnd) {
+ RECT r;
+ khm_handle csp_cw;
+ khm_handle csp_mw;
+ const wchar_t * wconfig;
+
+_report_cs0(KHERR_DEBUG_1, L"KPK mainwnd.c main_wnd_save_sizepos");
+ if (khm_main_wnd_mode == KHM_MAIN_WND_MINI)
+ wconfig = L"Windows\\MainMini";
+ else
+ wconfig = L"Windows\\Main";
+
+ GetWindowRect(hwnd, &r);
+
+ if (KHM_SUCCEEDED(khc_open_space(NULL,
+ L"CredWindow",
+ KHM_PERM_WRITE,
+ &csp_cw))) {
+ if (KHM_SUCCEEDED(khc_open_space(csp_cw,
+ wconfig,
+ KHM_PERM_WRITE,
+ &csp_mw))) {
+ khm_int32 t;
+
+ khc_write_int32(csp_mw, L"XPos", r.left);
+ khc_write_int32(csp_mw, L"YPos", r.top);
+ khc_write_int32(csp_mw, L"Width", r.right - r.left);
+ khc_write_int32(csp_mw, L"Height", r.bottom - r.top);
+
+ if (KHM_SUCCEEDED(khc_read_int32(csp_mw, L"Dock", &t)) &&
t != KHM_DOCK_NONE) {
+ khc_write_int32(csp_mw, L"Dock", KHM_DOCK_AUTO);
+ }
+
+ khc_close_space(csp_mw);
+ }
+ khc_close_space(csp_cw);
+ }
+}
+
LRESULT CALLBACK
khm_main_wnd_proc(HWND hwnd,
UINT uMsg,
@@ -544,17 +601,6 @@
}
break;

- case WM_MOVE:
- {
- SetTimer(hwnd,
- MW_RESIZE_TIMER,
- MW_RESIZE_TIMEOUT,
- NULL);
-
- return 0;
- }
- break;
-
case WM_MOVING:
{
RECT * r;
@@ -565,60 +611,17 @@
}
return TRUE;

+ case WM_EXITSIZEMOVE:
+ main_wnd_save_sizepos(hwnd);
+ break;
+
case WM_TIMER:
- if (wParam == MW_RESIZE_TIMER) {
- RECT r;
- khm_handle csp_cw;
- khm_handle csp_mw;
- const wchar_t * wconfig;
-
- if (khm_main_wnd_mode == KHM_MAIN_WND_MINI)
- wconfig = L"Windows\\MainMini";
- else
- wconfig = L"Windows\\Main";
-
- KillTimer(hwnd, wParam);
-
- GetWindowRect(hwnd, &r);
-
- if (KHM_SUCCEEDED(khc_open_space(NULL,
- L"CredWindow",
- KHM_PERM_WRITE,
- &csp_cw))) {
- if (KHM_SUCCEEDED(khc_open_space(csp_cw,
- wconfig,
- KHM_PERM_WRITE,
- &csp_mw))) {
- khm_int32 t;
-
- khc_write_int32(csp_mw, L"XPos", r.left);
- khc_write_int32(csp_mw, L"YPos", r.top);
- khc_write_int32(csp_mw, L"Width",
- r.right - r.left);
- khc_write_int32(csp_mw, L"Height",
- r.bottom - r.top);
-
- if (KHM_SUCCEEDED(khc_read_int32(csp_mw, L"Dock",
&t)) &&
- t != KHM_DOCK_NONE) {
- khc_write_int32(csp_mw, L"Dock",
KHM_DOCK_AUTO);
- }
-
- khc_close_space(csp_mw);
- }
- khc_close_space(csp_cw);
- }
-
- return 0;
-
- } else if (wParam == MW_REFRESH_TIMER) {
- kmq_post_message(KMSG_CRED, KMSG_CRED_REFRESH, 0, 0);
-
- return 0;
-
+ if (wParam == MW_REFRESH_TIMER) {
+ return kmq_post_message(KMSG_CRED, KMSG_CRED_REFRESH, 0,
0);
}
- break;

case WM_MENUSELECT:
return khm_menu_handle_select(wParam, lParam);

case KMQ_WM_DISPATCH:
Download (untitled) / with headers
text/plain 1.8KiB
[kpkoch - Tue Aug 7 13:53:02 2007]:

Show quoted text
> Actually, the bug was that the size & position were being remembered
> at the wrong time.
>
> The good news is that the patch below fixes the problem. The bad news
> is that it doesn't offer any insight into the display inconsistency
> bugs.

WM_EXITSIZEMOVE is unfortunately not a good message to rely on for
notifications of window size/position changes. It is used to notify a
window that it is exiting the moving/sizing modal loop. This loop is
used when the user is dragging a window around or using system menu and
cursor keys to reposition windows. However, windows can be repositioned
using other means as well, such as invoking the explorer window layout
commands (cascade windows, tile vertically/horizontally, etc..) and
third party desktop managers and some implementations of virtual desktop
managers. WM_MOVE is used to notify the window of size/position changes
in all these situations, which is why that message was triggering the
code which saves the window position. A problem with WM_MOVE is that if
the user preferences enable dragging the whole window during moves and
resizes, the window may receive multiple WM_MOVE messages in rapid
succession. This is also the case with some third party desktop
switchers that try to animate automatic window re-positioning. To
prevent Network Identity Manager from attempting to save each and every
intermediate window position, we don't directly save the position while
handling WM_MOVE. Instead we use a timer to wait until the torrent of
WM_MOVE messages end.

The actual bug was that we weren't saving the window position if the
user switched the display mode before this timeout expired. By the time
the timer fired, the window would be in a different position. A fix
would be to save the position and cancel the pending timer if a mode
switch is requested.

- Asanka Herath
Secure Endpoints Inc.
Download (untitled) / with headers
text/plain 3.6KiB
Patch:

Index: mainwnd.c
===================================================================
--- mainwnd.c (revision 19745)
+++ mainwnd.c (working copy)
@@ -205,6 +205,44 @@
}
}

+void main_wnd_save_sizepos(HWND hwnd) {
+ RECT r;
+ khm_handle csp_cw;
+ khm_handle csp_mw;
+ const wchar_t * wconfig;
+
+ if (khm_main_wnd_mode == KHM_MAIN_WND_MINI)
+ wconfig = L"Windows\\MainMini";
+ else
+ wconfig = L"Windows\\Main";
+
+ GetWindowRect(hwnd, &r);
+
+ if (KHM_SUCCEEDED(khc_open_space(NULL,
+ L"CredWindow",
+ KHM_PERM_WRITE,
+ &csp_cw))) {
+ if (KHM_SUCCEEDED(khc_open_space(csp_cw,
+ wconfig,
+ KHM_PERM_WRITE,
+ &csp_mw))) {
+ khm_int32 t;
+
+ khc_write_int32(csp_mw, L"XPos", r.left);
+ khc_write_int32(csp_mw, L"YPos", r.top);
+ khc_write_int32(csp_mw, L"Width", r.right - r.left);
+ khc_write_int32(csp_mw, L"Height", r.bottom - r.top);
+
+ if (KHM_SUCCEEDED(khc_read_int32(csp_mw, L"Dock", &t)) &&
t != KHM_DOCK_NONE) {
+ khc_write_int32(csp_mw, L"Dock", KHM_DOCK_AUTO);
+ }
+
+ khc_close_space(csp_mw);
+ }
+ khc_close_space(csp_cw);
+ }
+}
+
LRESULT CALLBACK
khm_main_wnd_proc(HWND hwnd,
UINT uMsg,
@@ -383,6 +424,8 @@

/* layout control */
case KHUI_ACTION_LAYOUT_MINI:
+ KillTimer(hwnd, MW_RESIZE_TIMER);
+ main_wnd_save_sizepos(hwnd);
if (khm_main_wnd_mode == KHM_MAIN_WND_MINI) {
khm_set_main_window_mode(KHM_MAIN_WND_NORMAL);
} else {
@@ -567,54 +610,11 @@

case WM_TIMER:
if (wParam == MW_RESIZE_TIMER) {
- RECT r;
- khm_handle csp_cw;
- khm_handle csp_mw;
- const wchar_t * wconfig;
-
- if (khm_main_wnd_mode == KHM_MAIN_WND_MINI)
- wconfig = L"Windows\\MainMini";
- else
- wconfig = L"Windows\\Main";
-
- KillTimer(hwnd, wParam);
-
- GetWindowRect(hwnd, &r);
-
- if (KHM_SUCCEEDED(khc_open_space(NULL,
- L"CredWindow",
- KHM_PERM_WRITE,
- &csp_cw))) {
- if (KHM_SUCCEEDED(khc_open_space(csp_cw,
- wconfig,
- KHM_PERM_WRITE,
- &csp_mw))) {
- khm_int32 t;
-
- khc_write_int32(csp_mw, L"XPos", r.left);
- khc_write_int32(csp_mw, L"YPos", r.top);
- khc_write_int32(csp_mw, L"Width",
- r.right - r.left);
- khc_write_int32(csp_mw, L"Height",
- r.bottom - r.top);
-
- if (KHM_SUCCEEDED(khc_read_int32(csp_mw, L"Dock",
&t)) &&
- t != KHM_DOCK_NONE) {
- khc_write_int32(csp_mw, L"Dock",
KHM_DOCK_AUTO);
- }
-
- khc_close_space(csp_mw);
- }
- khc_close_space(csp_cw);
- }
-
+ main_wnd_save_sizepos(hwnd);
return 0;
-
} else if (wParam == MW_REFRESH_TIMER) {
kmq_post_message(KMSG_CRED, KMSG_CRED_REFRESH, 0, 0);
-
return 0;
-
}
break;

Download (untitled) / with headers
text/plain 1.1KiB
[kpkoch - Tue Aug 7 22:19:05 2007]:

Show quoted text
> @@ -383,6 +424,8 @@
>
> /* layout control */
> case KHUI_ACTION_LAYOUT_MINI:
> + KillTimer(hwnd, MW_RESIZE_TIMER);
> + main_wnd_save_sizepos(hwnd);
> if (khm_main_wnd_mode == KHM_MAIN_WND_MINI) {
> khm_set_main_window_mode(KHM_MAIN_WND_NORMAL);
> } else {

This is not the only place that the main window mode is set. The mode
will also be implicitly switched if the main window is in basic mode and
the user requests a view that requires the advanced mode.

Ideally, killing the timer and saving the window position should be
performed in khm_set_main_window_mode(). This allows the rest of the
code to call this function without having to worry about saving the
window position, which is good because it's already being called from
multiple places.

Note that khm_set_main_window_mode() is called once before the main
window is actually created. Also, main_wnd_save_sizepos() relies on
khm_main_wnd_mode. So, killing the timer and saving the position should
happen before khm_main_wnd_mode is modified if khm_hwnd_main is non-NULL.

- Asanka Herath
Secure Endpoints Inc.
Download (untitled) / with headers
text/plain 4.2KiB
Show quoted text
> Ideally, killing the timer and saving the window position should be
> performed in khm_set_main_window_mode(). This allows the rest of the
> code to call this function without having to worry about saving the
> window position, which is good because it's already being called from
> multiple places.
>
> Note that khm_set_main_window_mode() is called once before the main
> window is actually created. Also, main_wnd_save_sizepos() relies on
> khm_main_wnd_mode. So, killing the timer and saving the position
> should
> happen before khm_main_wnd_mode is modified if khm_hwnd_main is non-
> NULL.

Point taken.

Patch:

Index: mainwnd.c
===================================================================
--- mainwnd.c (revision 19745)
+++ mainwnd.c (working copy)
@@ -205,6 +205,47 @@
}
}

+void main_wnd_save_sizepos() {
+ RECT r;
+ khm_handle csp_cw;
+ khm_handle csp_mw;
+ const wchar_t * wconfig;
+
+ if (!khm_hwnd_main) return;
+ KillTimer(khm_hwnd_main, MW_RESIZE_TIMER);
+
+ if (khm_main_wnd_mode == KHM_MAIN_WND_MINI)
+ wconfig = L"Windows\\MainMini";
+ else
+ wconfig = L"Windows\\Main";
+
+ GetWindowRect(khm_hwnd_main, &r);
+
+ if (KHM_SUCCEEDED(khc_open_space(NULL,
+ L"CredWindow",
+ KHM_PERM_WRITE,
+ &csp_cw))) {
+ if (KHM_SUCCEEDED(khc_open_space(csp_cw,
+ wconfig,
+ KHM_PERM_WRITE,
+ &csp_mw))) {
+ khm_int32 t;
+
+ khc_write_int32(csp_mw, L"XPos", r.left);
+ khc_write_int32(csp_mw, L"YPos", r.top);
+ khc_write_int32(csp_mw, L"Width", r.right - r.left);
+ khc_write_int32(csp_mw, L"Height", r.bottom - r.top);
+
+ if (KHM_SUCCEEDED(khc_read_int32(csp_mw, L"Dock", &t)) &&
t != KHM_DOCK_NONE) {
+ khc_write_int32(csp_mw, L"Dock", KHM_DOCK_AUTO);
+ }
+
+ khc_close_space(csp_mw);
+ }
+ khc_close_space(csp_cw);
+ }
+}
+
LRESULT CALLBACK
khm_main_wnd_proc(HWND hwnd,
UINT uMsg,
@@ -567,54 +611,11 @@

case WM_TIMER:
if (wParam == MW_RESIZE_TIMER) {
- RECT r;
- khm_handle csp_cw;
- khm_handle csp_mw;
- const wchar_t * wconfig;
-
- if (khm_main_wnd_mode == KHM_MAIN_WND_MINI)
- wconfig = L"Windows\\MainMini";
- else
- wconfig = L"Windows\\Main";
-
- KillTimer(hwnd, wParam);
-
- GetWindowRect(hwnd, &r);
-
- if (KHM_SUCCEEDED(khc_open_space(NULL,
- L"CredWindow",
- KHM_PERM_WRITE,
- &csp_cw))) {
- if (KHM_SUCCEEDED(khc_open_space(csp_cw,
- wconfig,
- KHM_PERM_WRITE,
- &csp_mw))) {
- khm_int32 t;
-
- khc_write_int32(csp_mw, L"XPos", r.left);
- khc_write_int32(csp_mw, L"YPos", r.top);
- khc_write_int32(csp_mw, L"Width",
- r.right - r.left);
- khc_write_int32(csp_mw, L"Height",
- r.bottom - r.top);
-
- if (KHM_SUCCEEDED(khc_read_int32(csp_mw, L"Dock",
&t)) &&
- t != KHM_DOCK_NONE) {
- khc_write_int32(csp_mw, L"Dock",
KHM_DOCK_AUTO);
- }
-
- khc_close_space(csp_mw);
- }
- khc_close_space(csp_cw);
- }
-
+ main_wnd_save_sizepos();
return 0;
-
} else if (wParam == MW_REFRESH_TIMER) {
kmq_post_message(KMSG_CRED, KMSG_CRED_REFRESH, 0, 0);
-
return 0;
-
}
break;

@@ -1114,6 +1116,8 @@

khui_refresh_actions();

+ main_wnd_save_sizepos();
+
khm_main_wnd_mode = mode;
if (khm_hwnd_main) {
khm_get_main_window_rect(&r);
This patch is functionally correct. There are some style issues:
(1) I prefer main_wnd_save_sizepos() to be static since it is not used
outside the source file.

(2) I prefer that main_wnd_save_sizepos() not be called from within
khm_set_main_window_mode() if the main application window has not been
created.

(3) the reason that main_wnd_save_sizepos() must be called before
khm_main_wnd_mode is updated is subtle and should be documented.

I will move this ticket to the krb5 queue and then commit the patch with
my style changes.
From: jaltman@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 1.3KiB
Patch developed by kpkoch with style changes from jaltman.

The size/position of the main application window is
internally updated in response to WM_MOVE messages but is
only written to the registry after a timeout period. This
is done due to the large number of WM_MOVE messages that
can be delivered during a windows drag / resize operation
involving the user or explorer shell's tile and cascade
operations. (or those involving third party desktop managers.)

In NIM 1.8 two different application view modes (standard
and advanced) replaced the single view mode in previous
releases. The size/position update logic was not modified
to take into consideration the possibility that a user might
move/resize the window and then quickly toggle modes before
the new location or size were recorded to the registry.

This change ensures that when a mode change occurs, via a
call to khm_set_main_window_mode(), that the current
location/size will be written to the registry and any
outstanding timer, MW_RESIZE_TIMER, will be cleared.

The logic to save the location/size has been extracted
into the new static function main_wnd_save_sizepos().

main_wnd_save_sizepos() is only called after the application
window has been created.


Commit By: jaltman



Revision: 19760
Changed Files:
U trunk/src/windows/identity/ui/mainwnd.c
From: tlyu@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 1.4KiB
pull up r19760 from trunk

r19760@cathode-dark-space: jaltman | 2007-08-08 13:45:37 -0400
ticket: 5613

Patch developed by kpkoch with style changes from jaltman.

The size/position of the main application window is
internally updated in response to WM_MOVE messages but is
only written to the registry after a timeout period. This
is done due to the large number of WM_MOVE messages that
can be delivered during a windows drag / resize operation
involving the user or explorer shell's tile and cascade
operations. (or those involving third party desktop managers.)

In NIM 1.8 two different application view modes (standard
and advanced) replaced the single view mode in previous
releases. The size/position update logic was not modified
to take into consideration the possibility that a user might
move/resize the window and then quickly toggle modes before
the new location or size were recorded to the registry.

This change ensures that when a mode change occurs, via a
call to khm_set_main_window_mode(), that the current
location/size will be written to the registry and any
outstanding timer, MW_RESIZE_TIMER, will be cleared.

The logic to save the location/size has been extracted
into the new static function main_wnd_save_sizepos().

main_wnd_save_sizepos() is only called after the application
window has been created.




Commit By: tlyu



Revision: 19798
Changed Files:
_U branches/krb5-1-6/
U branches/krb5-1-6/src/windows/identity/ui/mainwnd.c