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

Dmitry Timoshkov dmitry at baikal.ru
Wed Feb 17 07:15:34 CST 2016


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.

-- 
Dmitry.



More information about the wine-devel mailing list