2
votes

A recent article on The Old New Thing answers a question I've been wondering for a while: why my controls seem to lose focus after a global keystroke. So I go to implement the code there, adding error checks to the only function listed as returning an error, SetFocus(), as I do for all Windows API functions listed as returning a meaningful error, and... the error check triggers.

MSDN says

If the function succeeds, the return value is the handle to the window that previously had the keyboard focus. If the hWnd parameter is invalid or the window is not attached to the calling thread's message queue, the return value is NULL. To get extended error information, call GetLastError.

SetFocus() is returning NULL, which from this I assume means error, but when I examine the last error code, it's 0 (success). The focus change is working: if I remove the panic, the correct control is focused. So now I'm confused.

The sample program listed below gives you an edit control and a checkbox. Click the edit control, then switch programs and come back. You should see something like

result 00000000 new focus 0001004E

in the command line. Now check the checkbox and repeat, and you should see

panic: SetFocus() failed
last error: Success.

and the program should quit.

So my question is: what am I doing wrong or misunderstanding? Is NULL a valid return value for SetFocus() and MSDN is wrong? Or am I misreading what MSDN is trying to tell me and that NULL doesn't necessarily mean error? Or should I just not worry about the return value of SetFocus()?

Thanks.

// 5 june 2014
// scratch Windows program by pietro gagliardi 17 april 2014
// fixed typos and added toWideString() 1 may 2014
// borrows code from the scratch GTK+ program (16-17 april 2014) and from code written 31 march 2014 and 11-12 april 2014
#define _UNICODE
#define UNICODE
#define STRICT
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
#include <windows.h>

HMODULE hInstance;
HICON hDefaultIcon;
HCURSOR hDefaultCursor;
HFONT controlfont;

void panic(char *fmt, ...);
TCHAR *toWideString(char *what);

HWND entry;
HWND checkbox;
HWND lastfocus = NULL;

void onActivate(HWND hwnd, WPARAM wParam)
{
    UINT state = (UINT) LOWORD(wParam);
    int minimized = HIWORD(wParam) != 0;

    if (minimized)
        return;
    if (state == WA_INACTIVE) {
        HWND old;

        old = GetFocus();
        if (old != NULL && IsChild(hwnd, old))
            lastfocus = old;
    } else {
        int shouldpanic;
        HWND result;

        if (lastfocus == NULL)
            return;
        shouldpanic = SendMessage(checkbox, BM_GETCHECK, 0, 0) == BST_CHECKED;
        SetLastError(0);        // just in case
        result = SetFocus(lastfocus);
        if (shouldpanic && result == NULL)
            panic("SetFocus() failed");
        printf("result %p new focus %p\n", result, GetFocus());
    }
}

LRESULT CALLBACK wndproc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
    switch (msg) {
    case WM_ACTIVATE:
        onActivate(hwnd, wParam);
        return 0;
    case WM_CLOSE:
        PostQuitMessage(0);
        return 0;
    default:
        return DefWindowProc(hwnd, msg, wParam, lParam);
    }
    panic("oops: message %ud does not return anything; bug in wndproc()", msg);
}

HWND makeMainWindow(void)
{
    WNDCLASS cls;
    HWND hwnd;
    RECT r;

    ZeroMemory(&cls, sizeof (WNDCLASS));
    cls.lpszClassName = L"mainwin";
    cls.lpfnWndProc = wndproc;
    cls.hInstance = hInstance;
    cls.hIcon = hDefaultIcon;
    cls.hCursor = hDefaultCursor;
    cls.hbrBackground = (HBRUSH) (COLOR_BTNFACE + 1);
    if (RegisterClass(&cls) == 0)
        panic("error registering window class");
    r.left = 0;
    r.top = 0;
    r.right = 320;
    r.bottom = 70;
    if (AdjustWindowRectEx(&r, WS_OVERLAPPEDWINDOW, FALSE, 0) == 0)
        panic("AdjustWindowRectEx() failed");
    hwnd = CreateWindowEx(0,
        L"mainwin", L"Main Window",
        WS_OVERLAPPEDWINDOW,
        CW_USEDEFAULT, CW_USEDEFAULT,
        r.right - r.left, r.bottom - r.top,
        NULL, NULL, hInstance, NULL);
    if (hwnd == NULL)
        panic("opening main window failed");
    return hwnd;
}

void buildUI(HWND mainwin)
{
#define CSTYLE (WS_CHILD | WS_VISIBLE)
#define CXSTYLE (0)
#define SETFONT(hwnd) SendMessage(hwnd, WM_SETFONT, (WPARAM) controlfont, (LPARAM) TRUE);

    entry = CreateWindowEx(WS_EX_CLIENTEDGE | CXSTYLE,
        L"edit", L"Click here to focus",
        CSTYLE,
        10, 10, 300, 20,
        mainwin, (HMENU) 100, hInstance, NULL);
    if (entry == NULL)
        panic("error creating entry");
    SETFONT(entry);

    checkbox = CreateWindowEx(CXSTYLE,
        L"button", L"Panic",
        BS_AUTOCHECKBOX | CSTYLE,
        10, 40, 300, 20,
        mainwin, (HMENU) 101, hInstance, NULL);
    if (checkbox == NULL)
        panic("error creating checkbox");
    SETFONT(checkbox);
}

void initwin(void);
void firstShowWindow(HWND hwnd);

int main(int argc, char *argv[])
{
    HWND mainwin;
    MSG msg;

    initwin();

    mainwin = makeMainWindow();
    buildUI(mainwin);
    firstShowWindow(mainwin);

    for (;;) {
        BOOL gmret;

        gmret = GetMessage(&msg, NULL, 0, 0);
        if (gmret == -1)
            panic("error getting message");
        if (gmret == 0)
            break;
        TranslateMessage(&msg);
        DispatchMessage(&msg);
    }
    return 0;
}

void initwin(void)
{
    NONCLIENTMETRICS ncm;

    hInstance = GetModuleHandle(NULL);
    if (hInstance == NULL)
        panic("error getting hInstance");
    hDefaultIcon = LoadIcon(NULL, MAKEINTRESOURCE(IDI_APPLICATION));
    if (hDefaultIcon == NULL)
        panic("error getting default window class icon");
    hDefaultCursor = LoadCursor(NULL, MAKEINTRESOURCE(IDC_ARROW));
    if (hDefaultCursor == NULL)
        panic("error getting default window cursor");
    ncm.cbSize = sizeof (NONCLIENTMETRICS);
    if (SystemParametersInfo(SPI_GETNONCLIENTMETRICS,
        sizeof (NONCLIENTMETRICS), &ncm, 0) == 0)
        panic("error getting non-client metrics for getting control font");
    controlfont = CreateFontIndirect(&ncm.lfMessageFont);
    if (controlfont == NULL)
        panic("error getting control font");
}

void panic(char *fmt, ...)
{
    char *msg;
    TCHAR *lerrmsg;
    char *fullmsg;
    va_list arg;
    DWORD lasterr;
    DWORD lerrsuccess;

    lasterr = GetLastError();
    va_start(arg, fmt);
    vfprintf(stderr, fmt, arg);
    // according to http://msdn.microsoft.com/en-us/library/windows/desktop/ms680582%28v=vs.85%29.aspx
    lerrsuccess = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
        NULL, lasterr,
        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
        (LPTSTR) &lerrmsg, 0, NULL);
    if (lerrsuccess == 0) {
        fprintf(stderr, "critical error: FormatMessage() failed in panic(): last error in panic(): %ld, last error from FormatMessage(): %ld\n", lasterr, GetLastError());
        abort();
    }
    fwprintf(stderr, L"\nlast error: %s\n", lerrmsg);
    va_end(arg);
    exit(1);
}

void firstShowWindow(HWND hwnd)
{
    // we need to get nCmdShow
    int nCmdShow;
    STARTUPINFO si;

    nCmdShow = SW_SHOWDEFAULT;
    GetStartupInfo(&si);
    if ((si.dwFlags & STARTF_USESHOWWINDOW) != 0)
        nCmdShow = si.wShowWindow;
    ShowWindow(hwnd, nCmdShow);
    if (UpdateWindow(hwnd) == 0)
        panic("UpdateWindow(hwnd) failed in first show");
}
1
Thanks. I actually made this mistake in multiple places, but only fixed it in some! - andlabs
I'm not trying to attach another thread's message queue. This program runs in a single thread; there's no multiple threads. I'm not trying to talk to another app; this is just for when switching away from or back to this app. - andlabs
I think the bottom line has to be Raymond's oft-repeated advice (well, I think he's repeated it often): don't bother checking for an error if you don't know what to do about it. You weren't going to use the window handle that SetFocus returns so it doesn't matter whether you get one or not, and if you weren't able to change the focus for some other reason there's nothing you can do about it anyway. The code works, so just skip the error checking and think happy thoughts. :-) - Harry Johnston
@Harry: Agree wholeheartedly with your last comment regarding the bottom line. :-) I should have caught that from the code as well - I'll blame that on the cold meds as well. At least I was able to see that there was no call to AttachThreadInput() - it must have been before the meds kicked in. :-) - Ken White
@andlabs: if you'd like to do the experimentation necessary to confirm that SetFocus returns NULL (either with ERROR_SUCCESS or without setting any error code) precisely when the previously focused window isn't attached to the current thread, and post an answer to that effect, I for one would vote you up. :-) - Harry Johnston

1 Answers

1
votes

I decided to take Harry Johnston's advice to take Raymond Chen's advice not to worry about the error return because we can't handle the return somehow. All my error returns panic and abort the program anyway; I just wanted to cover all possible bases when it comes to error returns. (On GTK+ you can't override the error logging; on Mac OS X/Cocoa there are exceptions but some people yelled at me for asking about them, so I guess everyone's in agreement about all this...)

Here's a program that confirms that yes, SetFocus() returns NULL for an out-of-thread previous window:

// 6 june 2014
// based on winfocuspanictest 5 june 2014
// scratch Windows program by pietro gagliardi 17 april 2014
// fixed typos and added toWideString() 1 may 2014
// borrows code from the scratch GTK+ program (16-17 april 2014) and from code written 31 march 2014 and 11-12 april 2014
#define _UNICODE
#define UNICODE
#define STRICT
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdarg.h>
#include <windows.h>

HMODULE hInstance;
HICON hDefaultIcon;
HCURSOR hDefaultCursor;
HFONT controlfont;

void panic(char *fmt, ...);
TCHAR *toWideString(char *what);

HWND label;
HWND reportwin;

void onActivate(HWND hwnd, WPARAM wParam)
{
    UINT state = (UINT) LOWORD(wParam);
    int minimized = HIWORD(wParam) != 0;

    if (hwnd != reportwin)
        return;
    if (minimized)
        return;
    if (state == WA_INACTIVE) {
        HWND old;

        old = GetFocus();
        if (old != NULL && IsChild(hwnd, old))
            SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR) old);
    } else {
        HWND lastfocus, result;
        DWORD err;
        static WCHAR lastpos[200] = L"";

        lastfocus = (HWND) GetWindowLongPtr(hwnd, GWLP_USERDATA);
        if (lastfocus == NULL)
            return;
        SetLastError(0);        // just in case
        result = SetFocus(lastfocus);
        err = GetLastError();
        snwprintf(lastpos, 199, L"last: %p err: %d cur: %p", result, err, GetFocus());
        SendMessage(label, WM_SETTEXT, 0, (LPARAM) lastpos);
    }
}

LRESULT CALLBACK wndproc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
{
    switch (msg) {
    case WM_ACTIVATE:
        onActivate(hwnd, wParam);
        return 0;
    case WM_CLOSE:
        PostQuitMessage(0);
        return 0;
    default:
        return DefWindowProc(hwnd, msg, wParam, lParam);
    }
    panic("oops: message %ud does not return anything; bug in wndproc()", msg);
}

void makeMainWindowClass(void)
{
    WNDCLASS cls;

    ZeroMemory(&cls, sizeof (WNDCLASS));
    cls.lpszClassName = L"mainwin";
    cls.lpfnWndProc = wndproc;
    cls.hInstance = hInstance;
    cls.hIcon = hDefaultIcon;
    cls.hCursor = hDefaultCursor;
    cls.hbrBackground = (HBRUSH) (COLOR_BTNFACE + 1);
    if (RegisterClass(&cls) == 0)
        panic("error registering window class");
}

HWND makeMainWindow(int isreportwin)
{
    HWND hwnd;
    RECT r;

    r.left = 0;
    r.top = 0;
    r.right = 320;
    r.bottom = 70;
    if (AdjustWindowRectEx(&r, WS_OVERLAPPEDWINDOW, FALSE, 0) == 0)
        panic("AdjustWindowRectEx() failed");
    hwnd = CreateWindowEx(0,
        L"mainwin", L"Main Window",
        WS_OVERLAPPEDWINDOW,
        CW_USEDEFAULT, CW_USEDEFAULT,
        r.right - r.left, r.bottom - r.top,
        NULL, NULL, hInstance, NULL);
    if (hwnd == NULL)
        panic("opening main window failed");

#define CSTYLE (WS_CHILD | WS_VISIBLE)
#define CXSTYLE (0)
#define SETFONT(hwnd) SendMessage(hwnd, WM_SETFONT, (WPARAM) controlfont, (LPARAM) TRUE);

    HWND entry;
    HWND xlabel;

    entry = CreateWindowEx(WS_EX_CLIENTEDGE | CXSTYLE,
        L"edit", L"Click here to focus",
        CSTYLE,
        10, 10, 300, 20,
        hwnd, (HMENU) 100, hInstance, NULL);
    if (entry == NULL)
        panic("error creating entry");
    SETFONT(entry);

    xlabel = CreateWindowEx(CXSTYLE,
        L"static", L"",
        CSTYLE,
        10, 40, 300, 20,
        hwnd, (HMENU) 101, hInstance, NULL);
    if (xlabel == NULL)
        panic("error creating label");
    SETFONT(xlabel);

    if (isreportwin) {
        reportwin = hwnd;
        label = xlabel;
    }
    return hwnd;
}

void initwin(void);
void firstShowWindow(HWND hwnd);

DWORD WINAPI thread(LPVOID data)
{
    MSG msg;

    firstShowWindow(makeMainWindow(data != NULL));
    for (;;) {
        BOOL gmret;

        gmret = GetMessage(&msg, NULL, 0, 0);
        if (gmret == -1)
            panic("error getting message");
        if (gmret == 0)
            exit(0);        // kill the program if either window is closed
        TranslateMessage(&msg);
        DispatchMessage(&msg);
    }
    return 0;
}

int main(int argc, char *argv[])
{
    initwin();
    makeMainWindowClass();
    if (CreateThread(NULL, 0, thread, NULL, 0, NULL) == NULL)       // null for the other thread's window
        panic("error creating thread");
    thread((void *) thread);        // non-null for the main window
    return 0;
}

void initwin(void)
{
    NONCLIENTMETRICS ncm;

    hInstance = GetModuleHandle(NULL);
    if (hInstance == NULL)
        panic("error getting hInstance");
    hDefaultIcon = LoadIcon(NULL, MAKEINTRESOURCE(IDI_APPLICATION));
    if (hDefaultIcon == NULL)
        panic("error getting default window class icon");
    hDefaultCursor = LoadCursor(NULL, MAKEINTRESOURCE(IDC_ARROW));
    if (hDefaultCursor == NULL)
        panic("error getting default window cursor");
    ncm.cbSize = sizeof (NONCLIENTMETRICS);
    if (SystemParametersInfo(SPI_GETNONCLIENTMETRICS,
        sizeof (NONCLIENTMETRICS), &ncm, 0) == 0)
        panic("error getting non-client metrics for getting control font");
    controlfont = CreateFontIndirect(&ncm.lfMessageFont);
    if (controlfont == NULL)
        panic("error getting control font");
}

void panic(char *fmt, ...)
{
    char *msg;
    TCHAR *lerrmsg;
    char *fullmsg;
    va_list arg;
    DWORD lasterr;
    DWORD lerrsuccess;

    lasterr = GetLastError();
    va_start(arg, fmt);
    vfprintf(stderr, fmt, arg);
    // according to http://msdn.microsoft.com/en-us/library/windows/desktop/ms680582%28v=vs.85%29.aspx
    lerrsuccess = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
        NULL, lasterr,
        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
        (LPTSTR) &lerrmsg, 0, NULL);
    if (lerrsuccess == 0) {
        fprintf(stderr, "critical error: FormatMessage() failed in panic(): last error in panic(): %ld, last error from FormatMessage(): %ld\n", lasterr, GetLastError());
        abort();
    }
    fwprintf(stderr, L"\nlast error: %s\n", lerrmsg);
    va_end(arg);
    exit(1);
}

void firstShowWindow(HWND hwnd)
{
    // we need to get nCmdShow
    int nCmdShow;
    STARTUPINFO si;

    nCmdShow = SW_SHOWDEFAULT;
    GetStartupInfo(&si);
    if ((si.dwFlags & STARTF_USESHOWWINDOW) != 0)
        nCmdShow = si.wShowWindow;
    ShowWindow(hwnd, nCmdShow);
    if (UpdateWindow(hwnd) == 0)
        panic("UpdateWindow(hwnd) failed in first show");
}