Re: [PATCH] [comctl32] Fix sizing of edit window for listview icon view

[Nikolay Sivov]

On 25.09.2015 4:33, Barrett Karish wrote:
> ---
>   dlls/comctl32/listview.c | 42 +++++++++++++++++++++++++-----------------
>   1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c
> index e3fad51..ba227a7 100644
> --- a/dlls/comctl32/listview.c
> +++ b/dlls/comctl32/listview.c
> @@ -6048,12 +6048,15 @@ static LRESULT CALLBACK EditLblWndProcA(HWND hwnd, UINT uMsg, WPARAM wParam, LPA
>    */
>   static HWND CreateEditLabelT(LISTVIEW_INFO *infoPtr, LPCWSTR text, BOOL isW)
>   {
> -    static const DWORD style = WS_CHILDWINDOW|WS_CLIPSIBLINGS|ES_LEFT|ES_AUTOHSCROLL|WS_BORDER|WS_VISIBLE;
> -    HINSTANCE hinst = (HINSTANCE)GetWindowLongPtrW(infoPtr->hwndSelf, GWLP_HINSTANCE);
> +	DWORD style = WS_CHILDWINDOW | WS_CLIPSIBLINGS | ES_LEFT | ES_AUTOHSCROLL | WS_BORDER | WS_VISIBLE | ES_MULTILINE;
> +	HINSTANCE hinst = (HINSTANCE)GetWindowLongPtrW(infoPtr->hwndSelf, GWLP_HINSTANCE);

Hi, Barrett.

Please use existing 4 space formatting and avoid unnecessary changes.
> HWND hedit;
>
> TRACE("(%p, text=%s, isW=%d)\n", infoPtr, debugtext_t(text, isW), isW);
>
> +	if (infoPtr->uView == LV_VIEW_ICON)
> +		style |= ES_CENTER;

We need a test that proves that Edit gets centered style on icon view.
> +
>       /* window will be resized and positioned after LVN_BEGINLABELEDIT */
>       if (isW)
>   hedit = CreateWindowW(WC_EDITW, text, style, 0, 0, 0, 0, infoPtr->hwndSelf, 0, hinst, 0);
> @@ -6093,7 +6096,6 @@ static HWND LISTVIEW_EditLabelT(LISTVIEW_INFO *infoPtr, INT nItem, BOOL isW)
>       HFONT hOldFont = NULL;
>       TEXTMETRICW tm;
>       RECT rect;
> -    SIZE sz;
>       HDC hdc;
>
> TRACE("(nItem=%d, isW=%d)\n", nItem, isW);
> @@ -6152,24 +6154,30 @@ static HWND LISTVIEW_EditLabelT(LISTVIEW_INFO *infoPtr, INT nItem, BOOL isW)
>       GetWindowTextW(infoPtr->hwndEdit, disptextW, DISP_TEXT_SIZE);
>       TRACE("edit box text=%s\n", debugstr_w(disptextW));
>
> -    /* get string length in pixels */
> -    GetTextExtentPoint32W(hdc, disptextW, lstrlenW(disptextW), &sz);
> +	DrawTextW(hdc, disptextW, -1, &rect, infoPtr->uView == LV_VIEW_ICON ? LV_FL_DT_FLAGS : LV_SL_DT_FLAGS | DT_CALCRECT);
>
> -    /* add extra spacing for the next character */
> -    GetTextMetricsW(hdc, &tm);
> -    sz.cx += tm.tmMaxCharWidth * 2;
> +	if (infoPtr->uView != LV_VIEW_ICON)
> +	{
> +		/* add extra spacing for the next character */
> +		GetTextMetricsW(hdc, &tm);
> +		rect.right += tm.tmMaxCharWidth;
> +		rect.left -= 2;
> +		rect.top -= 1;
> +		rect.bottom += 2;
> +	}
> +	else
> +	{
> +		rect.bottom += 3;
> +	}

This should ideally use some font metrics instead of hardcoded magic, 
but I know we do it this way already.
> -    if (infoPtr->hFont)
> -        SelectObject(hdc, hOldFont);
> +	if (infoPtr->hFont)
> +		SelectObject(hdc, hOldFont);
>
> -    ReleaseDC(infoPtr->hwndSelf, hdc);
> +	ReleaseDC(infoPtr->hwndSelf, hdc);
>
> -    sz.cy = rect.bottom - rect.top + 2;
> -    rect.left -= 2;
> -    rect.top  -= 1;
> -    TRACE("moving edit=(%d,%d)-(%d,%d)\n", rect.left, rect.top, sz.cx, sz.cy);
> -    MoveWindow(infoPtr->hwndEdit, rect.left, rect.top, sz.cx, sz.cy, FALSE);
> -    ShowWindow(infoPtr->hwndEdit, SW_NORMAL);
> +	TRACE("moving edit=(%d,%d)-(%d,%d)\n", rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top);
> +	MoveWindow(infoPtr->hwndEdit, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, FALSE);
> +	ShowWindow(infoPtr->hwndEdit, SW_NORMAL);
>       SetFocus(infoPtr->hwndEdit);
>       SendMessageW(infoPtr->hwndEdit, EM_SETSEL, 0, -1);
>       return infoPtr->hwndEdit;