user32: Use top level window as dialog parent for modal dialogs.

Dmitry Timoshkov dmitry at baikal.ru
Tue Apr 5 08:04:58 CDT 2016


Jacek Caban <jacek at codeweavers.com> wrote:

> >>>> This is part of the fix for bug 40282.
> >>> Could you please add actual tests that show the handling of disabled
> >>> window state? Neither current tests nor the new ones actually check
> >>> that at all, having such tests would prevent new regressions in this
> >>> area. Feel free to use my tests from the staging repository as a base
> >>> if desired, or write your own ones that use the same approach.
> >> This patch is not directly related to enabled/disabled state of
> >> parent/owner. I plan to send such tests together with the other part of
> >> the fix.
> > It still looks like shooting in the dark to me without writing the tests
> > first and only then starting to fix the test failures one by one, so it
> > becomes absolutely clear what exactly is fixed and where. Once again I'd
> > like to point out that writing exhaustive tests before starting to add
> > any fixes is very important for better understanding of the problem,
> > especially in such fargile areas as user32 window management code.
> 
> Actually, my patch shows that we handle parent window in a wrong way.
> Period. That's what's obviously wrong and that's what I'm fixing at the
> moment. I don't touch enabling and disabling owner in this patch. In
> fact, it shouldn't change it at all. So please, forget about context of
> this bug, do you still have any comment about this particular patch?

It seems that we heard something similar when you sent the patches last
time that have caused this regression. And absense of good tests in both
cases doesn't add a lot of confidence that there will be no new regressions.

> >> For the reference, I attached a patch that (on top on this one)
> >> passes all your tests.
> > I have more tests that show that removing DF_OWNERENABLED handling altogether
> > (like my own patches also do) is wrong. These tests are not part of the staged
> > patch set.
> 
> Yep, that's why I only attached it for the reference and didn't send it
> to wine-patches. It needs more tests and I don't consider your test good
> enough.

Probably my tests are not good for you, but please show your tests first,
so we could make a comparison. Btw, it's pretty clear to me that you used
my tests as a reference for of your test, if my tests were that bad you
wouldn't do that, right? :)

-- 
Dmitry.



More information about the wine-devel mailing list