Re: comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

[Vitaliy Margolen]

On 05/09/2011 01:09 AM, Nikolay Sivov wrote:
> Fix for http://bugs.winehq.org/show_bug.cgi?id=25908

There are 3 separate changes in this patch:
> - BOOL bSame;
> - BOOL same;

Variable name rename.

The actual fix with new test.
> - ok(g_editbox_disp_info.item.pszText != NULL, "got %p\n", g_editbox_disp_info.item.pszText);

Btw it's nice to print what did you expected back.
> - ok(!IsWindow(hwndedit), "Expected Edit window to be freed\n");

I'm not so sure you can rely on system not allocating any more windows with 
the same handle. This is a race condition.
> - /* WM_COMMAND with EN_KILLFOCUS isn't forwared to parent */
> - /* WM_COMMAND with EN_KILLFOCUS isn't forwarded to parent */

A typo fix.

Vitaliy.

comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

[Nikolay Sivov]

On 5/9/2011 17:39, Vitaliy Margolen wrote:
> On 05/09/2011 01:09 AM, Nikolay Sivov wrote:
> > Fix for http://bugs.winehq.org/show_bug.cgi?id=25908
>
> There are 3 separate changes in this patch:
> > - BOOL bSame;
> > - BOOL same;
>
> Variable name rename.

I don't think it's a big deal.
> The actual fix with new test.
> > - ok(g_editbox_disp_info.item.pszText != NULL, "got %p\n",
> >
> > g_editbox_disp_info.item.pszText);
>
> Btw it's nice to print what did you expected back.

I don't expect NULL, that's all. I don't care about particular pointer 
value. If you mean I should print that I expect NULL, I don't think so, 
it's redundant. In case it fails you need to look at failed line anyway.
> > - ok(!IsWindow(hwndedit), "Expected Edit window to be freed\n");
>
> I'm not so sure you can rely on system not allocating any more windows 
> with the same handle. This is a race condition.

This duplicates previous test behaviour that's doing just fine for quite 
a long time. Test is single threaded, and you should run test program in 
clean environment, with no programs around best. So it's working fine in 
practice.
> > - /* WM_COMMAND with EN_KILLFOCUS isn't forwared to parent */
> > - /* WM_COMMAND with EN_KILLFOCUS isn't forwarded to parent */
>
> A typo fix.

And? Separate patch to fix a typo in test file comment? (same test 
function I'm patching now even)
> Vitaliy.

comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

[Vitaliy Margolen]

I'm guessing AJ doesn't care much about such details. So NVM.

Anyone want's to remove any requirements to wine tests? As long as they 
compile and pass on AJ's pc, is seems to be the only requirement.

Vitaliy.

On 05/09/2011 08:42 AM, Nikolay Sivov wrote:
> On 5/9/2011 17:39, Vitaliy Margolen wrote:
> > On 05/09/2011 01:09 AM, Nikolay Sivov wrote:
> > > Fix for http://bugs.winehq.org/show_bug.cgi?id=25908
> >
> > There are 3 separate changes in this patch:
> > > - BOOL bSame;
> > > - BOOL same;
> >
> > Variable name rename.
>
> I don't think it's a big deal.
> > The actual fix with new test.
> > > - ok(g_editbox_disp_info.item.pszText != NULL, "got %p\n",
> > >
> > > g_editbox_disp_info.item.pszText);
> >
> > Btw it's nice to print what did you expected back.
>
> I don't expect NULL, that's all. I don't care about particular pointer
> value. If you mean I should print that I expect NULL, I don't think so, it's
> redundant. In case it fails you need to look at failed line anyway.
> > > - ok(!IsWindow(hwndedit), "Expected Edit window to be freed\n");
> >
> > I'm not so sure you can rely on system not allocating any more windows
> > with the same handle. This is a race condition.
>
> This duplicates previous test behaviour that's doing just fine for quite a
> long time. Test is single threaded, and you should run test program in clean
> environment, with no programs around best. So it's working fine in practice.
> > > - /* WM_COMMAND with EN_KILLFOCUS isn't forwared to parent */
> > > - /* WM_COMMAND with EN_KILLFOCUS isn't forwarded to parent */
> >
> > A typo fix.
>
> And? Separate patch to fix a typo in test file comment? (same test function
> I'm patching now even)
> > Vitaliy.

comctl32/listview: Make edit box handle available from LVN_ENDLABELEDIT notification

[Alexandre Julliard]

Vitaliy Margolen  writes:
> I'm guessing AJ doesn't care much about such details. So NVM.
>
> Anyone want's to remove any requirements to wine tests? As long as
> they compile and pass on AJ's pc, is seems to be the only requirement.

The tests are supposed to succeed. Writing fancy failure messages is a
waste of time, since they are not supposed to be seen anyway. Printing
the failing value is useful, but printing "foo was expected instead" is
generally useless.

And about window handles, IsWindow() is just fine, handles don't get
reused the way you think. Learn how things work before flaming others.