[PATCH 2/2 v3] user32: Add more dialog creation message tests

[Alexandre Julliard]

Dmitry Timoshkov <dmitry at baikal.ru> writes:

> Alexandre Julliard <julliard at winehq.org> wrote:
>
>> > Piotr Caban <piotr at codeweavers.com> wrote:
>> >> But it doesn't prove anything. It's something like: it's not possible to 
>> >> test if something is done in this place so lets test everything else to 
>> >> make sure it's not done there. Such tests can be added to test 
>> >> DefDlgProc, not to show that my patch is valid.
>> >> I'm not planning to add more tests related to this code. I think that 
>> >> this change is quite obvious and already well tested.
>> >
>> > Many of the tests in msg.c and win.c are created by me, I know the related
>> > code in user32 pretty well, and based on my knowledge I can't agree that
>> > your change is obvious, at least it's absolutely not obvious to me. Focus
>> > handling is very fragile part of user32 code, there are applications that
>> > are very sensitive to any change in that area, and it's very easy to break
>> > them. Please take time to create a more convincing set of tests.
>> 
>> The focus handling is definitely not obvious, but the fact that setting
>> the focus should also select the edit control seems pretty clear from
>> the message trace, and it matches what DefDlgProc already does in other
>> situations. What is it that you find not convincing here?  Are you
>> saying that the whole initial focus setting should be moved to
>> DefDlgProc?
>
> What makes me hesitant is the fact that text selection logic is basically
> duplicated in several places of the dialog management code. So, I'd like
> to know whether the dialog creation code needs somehow route part of that
> logic to DefDlgProc(), and if that's the case perhaps what is going on is
> that DefDlgProc() should be called as a part of dialog creation process.

I've added a test that shows that DefDlgProc is not involved here. If
you still find it unconvincing, please let me know what other tests
you'd like to see.

-- 
Alexandre Julliard
julliard at winehq.org