[3/5] comctl32: Implement TaskDialogIndirect as a custom dialog box

Nikolay Sivov bunglehead at gmail.com
Thu Feb 19 06:51:32 CST 2015


On 19.02.2015 15:23, Joachim Priesner wrote:
> +        WCHAR *windowTitleBuffer = NULL;
> +        UINT len = LoadStringW(config->hInstance, LOWORD(config->pszWindowTitle), (LPWSTR)&ptr, 0);
> +        if (len)
> +        {
> +            windowTitleBuffer = Alloc((len + 1) * sizeof(WCHAR));
> +            if (!windowTitleBuffer)
> +            {
> +                EndDialog(hwnd, E_OUTOFMEMORY);
> +                return;
> +            }
> +            memcpy(windowTitleBuffer, ptr, len * sizeof(WCHAR));
> +            windowTitleBuffer[len] = 0;
> +            SetWindowTextW(hwnd, windowTitleBuffer);
> +        }

That's essentially the same code you're using to set IDC_CONTENT text. 
Could you add a helper that does all of that? Also it will probably take 
fewer lines to use LoadStringW() to fill your buffer and set terminating 
null.

> +    /* Destroy unused common buttons. */
> +    TRACE("dwCommonButtons=%x\n", config->dwCommonButtons);
> +    if (!(config->dwCommonButtons & TDCBF_YES_BUTTON))
> +        DestroyWindow(GetDlgItem(hwnd, IDYES));

Did you check that it's actually destroyed and not just hidden?

> +            WCHAR buttonText[1024];
> +            int w, h;
> +            if (GetWindowTextW(hItem, buttonText, 1024))

Where does this limit come from?

> +    /* Query parent window/desktop size and center window. */
> +    monitor = MonitorFromWindow(config->hwndParent ? config->hwndParent : GetActiveWindow(),
> +                                MONITOR_DEFAULTTOPRIMARY);
> +    monitorInfo.cbSize = sizeof(monitorInfo);
> +    GetMonitorInfoW(monitor, &monitorInfo);
> +    windowLeft = (monitorInfo.rcWork.left + monitorInfo.rcWork.right
> +                  - (windowClientWidth + windowBorderWidth)) / 2;
> +    windowTop = (monitorInfo.rcWork.top + monitorInfo.rcWork.bottom
> +                 - (windowClientHeight + windowBorderHeight)) / 2;
> +    SetWindowPos(hwnd, 0, windowLeft, windowTop,
> +                 windowClientWidth + windowBorderWidth, windowClientHeight + windowBorderHeight,
> +                 SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOREDRAW);

This looks overcomplicated, isn't it enough to use desktop window if no 
parent was specified?

> +    DeleteObject(mainInstructionFont);
> +    Free(contentBuffer);
> +    Free(mainInstructionBuffer);

It's better to free right when you don't need it anymore, and please use 
simpler/shorter names.



More information about the wine-devel mailing list