Re: [1/2] comctl32/tests: Added tests for mouse events handling (try 3)
Daniel Jelinski <djelinski1(a)gmail.com> wrote:
Skipping tests on WinNT - older versions of comctl send a different set of events.
Then you need to figure out what is different, and make the tests pass there as well (there is 'optional' flag for that), otherwise you risk breaking applications written for NT.
+ /* WM_RBUTTONDOWN does not return until it gets another mouse event. + Make sure it gets one by posting WM_RBUTTONUP to message queue */ + PostMessageA(hTree, WM_RBUTTONUP, 0, (LPARAM)0x10001); + /* this sequence should NOT send WM_CONTEXTMENU */ + SendMessageA(hTree, WM_RBUTTONDOWN, 2, (LPARAM)0x10001); + + /* ditch the first sequence as it processed more messages than we need. Redo */ + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + PostMessageA(hTree, WM_RBUTTONUP, 0, (LPARAM)0x10001); + SendMessageA(hTree, WM_RBUTTONDOWN, 2, (LPARAM)0x10001);
This can't work. If you use PostMessage you need to flush message queue before testing the sequence. Using SendMessage after PostMessage doesn't guarantee that a posted message is handled before a sent one, you may try generating hardware messages instead.
+ + if(sequences[PARENT_SEQ_INDEX]->sequence->message == 0x133) + { + win_skip("Comctl32 versions prior to 5.80 send different set of events"); + return; + }
Please use symbolic names for messages instead of magic numbers. -- Dmitry.
2012/4/8 Dmitry Timoshkov <dmitry(a)baikal.ru>:
Daniel Jelinski <djelinski1(a)gmail.com> wrote:
Skipping tests on WinNT - older versions of comctl send a different set of events.
Then you need to figure out what is different, and make the tests pass there as well (there is 'optional' flag for that), otherwise you risk breaking applications written for NT.
Okay, I'll do that. I thought that since comctl is supposed to be backwards compatible, I'll write the tests to just pass on the newer version.
+ /* WM_RBUTTONDOWN does not return until it gets another mouse event. + Make sure it gets one by posting WM_RBUTTONUP to message queue */ + PostMessageA(hTree, WM_RBUTTONUP, 0, (LPARAM)0x10001); + /* this sequence should NOT send WM_CONTEXTMENU */ + SendMessageA(hTree, WM_RBUTTONDOWN, 2, (LPARAM)0x10001); + + /* ditch the first sequence as it processed more messages than we need. Redo */ + flush_sequences(sequences, NUM_MSG_SEQUENCES); + + PostMessageA(hTree, WM_RBUTTONUP, 0, (LPARAM)0x10001); + SendMessageA(hTree, WM_RBUTTONDOWN, 2, (LPARAM)0x10001);
This can't work. If you use PostMessage you need to flush message queue before testing the sequence. Using SendMessage after PostMessage doesn't guarantee that a posted message is handled before a sent one, you may try generating hardware messages instead.
I want the posted message to be handled after the sent one, and I get exactly that. The event handler for WM_RBUTTONDOWN captures all messages until it finds one of the mouse-related events, so I needed to post WM_RBUTTONUP first to make sure that SendMessage returns. Hardware messages are an alternative, but I couldn't find any relevant examples, so I followed the path of least resistance. The first PostMessage/SendMessage sequence flushes the message queue, or at least its interesting part. There are probably better ways (PeekMessage/DispatchMessage loop comes to mind), but this one just works.
+ + if(sequences[PARENT_SEQ_INDEX]->sequence->message == 0x133) + { + win_skip("Comctl32 versions prior to 5.80 send different set of events"); + return; + }
Please use symbolic names for messages instead of magic numbers.
Ok, can do. Best regards, Daniel
On 04/08/2012 01:44 PM, Daniel Jelinski wrote:
Hardware messages are an alternative, but I couldn't find any relevant examples, so I followed the path of least resistance. These tests you wrote won't work. Emulating mouse up/down events with anything other then hardware messages will not exercise the entire chain. For example search for "mouse_event" or SendInput.
Vitaliy.
Dnia 09-04-2012 o 04:13:44 Vitaliy Margolen <wine-devel(a)kievinfo.com> napisał(a):
On 04/08/2012 01:44 PM, Daniel Jelinski wrote:
Hardware messages are an alternative, but I couldn't find any relevant examples, so I followed the path of least resistance. These tests you wrote won't work. Emulating mouse up/down events with anything other then hardware messages will not exercise the entire chain. For example search for "mouse_event" or SendInput.
Thank you for your feedback. I agree, the tests are not equivalent to real mouse input. However they do expose the differences between WinAPI and Wine. I wrote them just to make sure (and prove) that my fix to comctl implementation does the right thing, and that I do not introduce new bugs in place of the old one. If I remove the changes to tests and post changes to the library only, will they get accepted? Daniel
Hello, I see that my patch hasn't been accepted yet. Well if some explanation could help it through, here goes: Bug 19222 makes using MS SQL Server management studio a pain. The application displays database structure in a tree view, and most actions are executed from a context menu displayed by right-clicking on a relevant position in the tree view. Unfortunately Wine suffers from a bug - right click displays the menu, but selecting any option from the menu causes another menu to be displayed at the current mouse position. Selecting an option from this second menu works as expected. Also opening the popup menu by pressing context menu key on the keyboard works properly. The usual suspect in such cases is that incorrect messages are sent, or messages are sent in incorrect order. I tried to run the application with native comctl, but unfortunately it refused to work - probably comctl version 6.0 is required, while winetricks installs version 5.80. So I created my own application in Delphi. It dispayed the same problem with builtin comctl and worked correctly with native. Then I tested it with WINEDEBUG=+message. The outstanding differences were: - on native comctl treeview sent two messages to the main window - first WM_NOTIFY (NM_RCLICK) and then WM_CONTEXTMENU - on builtin comctl treeview sent first WM_CONTEXTMENU to self, which then got propagated to main window, and then WM_NOTIFY(NM_RCLICK). There was something peculiar - WM_RBUTTONUP never appeared in the logs. I checked to see why and found a function TREEVIEW_TrackMouse. This function is called by WM_RBUTTONDOWN handler and it captures all events from the message queue until it finds either WM_RBUTTONUP or WM_MOUSEMOVE with coordinates sufficiently far from the origin of the click. There was also a comment stating that "This is quite unusual piece of code, but that's how it's implemented in Windows.". Given the absence of WM_RBUTTONUP in message logs I consider this assertion to be true. With this knowledge I proceeded to write tests. I wanted to emulate right button down/up sequence. I couldn't send both messages by SendMessage, because SendMessage sends messages directly to the window procedure, and WM_RBUTTONUP has to be delivered by message queue. I also didn't want to push both messages to the queue - this would net me several more irrelevant messages like WM_PAINT and could cause failures if someone moved the (physical) mouse during the test. This is why I chose to post WM_RBUTTONUP and then send WM_RBUTTONDOWN. Later, thanks to our friendly bot Marvin, I found out that older versions of comctl sent WM_PAINT messages during the button down/button up sequence, bypassing the message queue. It took me several tries to find out which messages are sent, but now the patch works correctly on all tested comctl versions. Then I corrected Wine to pass the tests. I tested both applications (Microsoft's and mine) to see if they work correctly when the correct messages are sent. They do. If there is anything else I can do to help this patch get accepted, let me know. I'd like to have a clean git origin before I start fixing other treeview problems. Thanks for your patience, Daniel
On Sun, Apr 15, 2012 at 07:38, Daniel Jelinski <djelinski1(a)gmail.com> wrote:
Hello, ... Bug 19222 makes using MS SQL Server management studio a pain. The application displays database structure in a tree view, and most actions are executed from a context menu displayed by right-clicking on a relevant position in the tree view. Unfortunately Wine suffers from a bug - right click displays the menu, but selecting any option from the menu causes another menu to be displayed at the current mouse position. Selecting an option from this second menu works as expected.
This problem is also seem in FlashFXP so your patch may fix it there as well. http://bugs.winehq.org/show_bug.cgi?id=19350#c4
Thanks for your patience, Daniel
Bruno
participants (5)
-
Bruno Jesus -
Daniel Jelinski -
Daniel Jeliński
-
Dmitry Timoshkov -
Vitaliy Margolen