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?
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.
Regards, Sebastian