Re: [PATCH] user32: Handle VK_RETURN KeyDown events more properly in EDITcontrols.

[Reece Dunn]

On 19/04/2008, El.  wrote:
> > "El."  wrote:
> > > @@ -4606,6 +4606,10 @@ static LRESULT EDIT_WM_KeyDown(EDITSTATE *es, INT key)
> > >      {
> > >    HWND hwndParent = GetParent(es->hwndSelf);
> > >    DWORD dw = SendMessageW( hwndParent, DM_GETDEFID, 0, 0 );
> > > -
> > > - if (GetClassLongW(hwndParent, GCW_ATOM) != WC_DIALOG)
> > > -
> > >  break;
> >
> > You need to add a test case first to confirm that the fix is correct, and
> > use EDIT_IsInsideDialog to detect if the parent is dialog.
>
> Well I'm not sure what to test in the first place, I didn't assume that
> modifying some aspect of the code to be in compliance with official
> "standards" required writing any test case.
>
> I might be missing what you mean by the "correctness" of the fix,
> since I'm not really familiar with Wine's patching practices.
>
> http://msdn2.microsoft.com/en-us/library/bb775464(VS.85).aspx

The MSDN documentation is a good source of information for how various
controls and APIs behave. However, this is only a starting point.

The aim of Wine is to match the behaviour of Windows, not the
documented behaviour. While the patch is likely to be correct, this is
not always the case.

The test case will be run on (and verified against) the different
versions of Windows. This will also test other things not explicitly
documented: what is the return value; is GetLastError set; what is the
sequence of messages sent in each case? All of these will add weight
to your fix.

The tests also serve another purpose. They allow the developers to
refactor, rewrite or address other issues in the code and have a
greater confidence that they haven't broken anything. If a bug is
reported and then fixed, if there isn't a valid test case for that
bug, there is a chance that the bug will reappear later on.

When you have documentation (such as MSDN) that describes behaviour
that is not supported in Wine, or differs in some way, the approach
should be to write tests to validate the documentation and then
implement it. That way, you have the verification that what you have
written is correct.

Thank you for improving Wine,
- Reece