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

Jacek Caban jacek at codeweavers.com
Tue Apr 5 11:42:44 CDT 2016


Hi Sebastian,

On 04/05/16 17:25, Sebastian Lackner wrote:
> On 05.04.2016 16:28, Jacek Caban wrote:
>> Hi Dmitry,
>>
>> I will ask you one more time to go back to the code and being technical
>> instead of personal. Take a look at the code and show me a code path,
>> that makes enabling or disabling any different than before the patch. If
>> you can't then you will understand that the change is unrelated. If you
>> do, I'm happy to write a test for that case.
>>
>> Jacek
>>
>>
> To both of you, please calm down a bit. Whereas there is nothing obviously wrong
> with the patch, I can also understand Dmitrys request to merge more tests first,
> before we come to the conclusion that this solution must be right. In fact, when
> taking into account the feedback I received quite often to my own patches, the
> second approach (first ensure tests cover all areas, then changing the
> implementation) is indeed preferred for Wine, so I'm not really sure what the
> problem is here. How should Dmitry add his own tests when you want to send all
> in a single patch? Since Dmitry already did the effort to track down your
> regression, wouldn't it be useful to trust him a bit more and to cooperate?

To be honest, I don't. I said it multiple time: his requests are
unrelated. I will send those tests when they will be related to the
patch I send. In fact I'd be writing them now if I wouldn't have to deal
with this nonsense. But still and again, this it's unrelated to this
patch, so this stays as is. Let's keep it separated, I will try to
explain it one last time in a different mail.

> Imho, what we especially need to ensure is that wineserver handling for various
> flag combinations is really correct, before we modify the owner on user32 side.
> Based on some tests I did a while ago (when handing for WS_POPUP | WS_CHILD was
> added), there are definitely remaining todos. Also please note that neither your
> old nor your new code correctly replicates wineserver behaviour because
> WIN_CreateWindowEx contains a check: if ((cs->style & (WS_CHILD|WS_POPUP)) != WS_CHILD)
> Under specific circumstances the owner argument is simply ignored.

Yes, it's intentionally different, because my tests show it should be
different. If we pass WS_CHILD, then the argument that we pass as owner
is in fact a parent. A parent does not have the requirements of being
top level window (or one with no WS_CHILD flag to be precise). Otherwise
it's an owner, which has the restriction. That's how CreateWindow works,
it's tested and fixed by [1]. Tests that I included in the patch show
that dialogs created by DialogBoxIndirectParam are different. They have
the same restriction applied to the case with WS_CHILD as if it was an
owner. My tests clearly show that. So to conclude:
- I did fix todos you talk about. At least ones I know about.
- I did add test that proves it's different in this case and that it has
to be done in user32.
Does that sound right to you?

Jacek


[1]
http://source.winehq.org/git/wine.git/commitdiff/d13a44e4aa12fa5ad3498c39934e88a7dcd30916?hp=47b2238b3dc7ad5f0f482d0c9cd525cfeb4537f5




More information about the wine-devel mailing list