[Bug 13112] New: comctl32 trackbar setPos() should not call oncustomdraw if value hasn't changed
http://bugs.winehq.org/show_bug.cgi?id=13112
Summary: comctl32 trackbar setPos() should not call oncustomdraw if value hasn't changed Product: Wine Version: CVS/GIT Platform: PC OS/Version: Linux Status: UNCONFIRMED Severity: minor Priority: P2 Component: comctl32 AssignedTo: wine-bugs@winehq.org ReportedBy: jaz@pastnotecut.org
Created an attachment (id=12893) --> (http://bugs.winehq.org/attachment.cgi?id=12893) patch against comctl32 for the setpos bug
The SetPos() function of the trackbar component of comctl32 is calling the OnCustomDrawMessage even if it doesn't change the value.
This wouldn't be a problem per se, but in my application I was doing a setFocus inside the oncustomdraw, as well as doing the setpos in a timer. This caused all the buttons in the UI to not operate, because the timer took precedence, and the focus no longer kept at the supposed place. I've already fixed this in my application (since i can do the check), but could affect other applications.
I attach the patch that i've tested which solves it.
http://bugs.winehq.org/show_bug.cgi?id=13112
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch
--- Comment #1 from Austin English austinenglish@gmail.com 2008-05-11 22:34:14 --- Please send patches to wine-patches@winehq.org
http://bugs.winehq.org/show_bug.cgi?id=13112
Mikolaj Zalewski mikolaj.zalewski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mikolaj.zalewski@gmail.com
--- Comment #2 from Mikolaj Zalewski mikolaj.zalewski@gmail.com 2008-07-05 13:44:47 --- I see that a mail was sent to wine-patches but the patch wasn't applied. This could be because... you didn't attach the patch to the mail :). Also you should make the subject more descriptive for a changelog entry - e.g. 'comctl32: trackbar: make sure a MN_CUSTOMDRAW is no sent after a TBM_SETPOS'. In the message body you can add that it fixes the problem in bug #13112 so that people see you are fixing a real problem and not nitpicking.
The patch itself seems correct. Of course, it would be great if you would also include a test added to dlls/comctl32/tests/trackbar.c.
http://bugs.winehq.org/show_bug.cgi?id=13112
--- Comment #3 from Josep Ma jaz@pastnotecut.org 2008-09-27 07:37:09 --- I see this patch hasn't been applied yet to the repository. What is still missing? I sent an e-mail as per your request on july 6, with subject : comctl32:,trackbar: make sure a MN_CUSTOMDRAW is not sent for calls to TBM_SETPOS with the current value
and the patch attached correctly this time. What i've just realized is that i sent it with another e-mail account instead of the one here. Would that have caused it to be ignored?
http://bugs.winehq.org/show_bug.cgi?id=13112
--- Comment #4 from Lei Zhang thestig@google.com 2008-09-27 17:33:00 --- It would be good to write a test case in dlls/comctl32/tests/trackbar.c and make sure it passes on Windows. To prove your patch is correct.
http://bugs.winehq.org/show_bug.cgi?id=13112
--- Comment #5 from Josep Ma jaz@pastnotecut.org 2008-09-28 10:10:23 --- I don't know how to test this. The current tests check that the call produces a specific result.
In this case, what matters is the call the function itself does to Invalidate, which is not required. This doesn't change the trackbar in itself, so I wouldn't know how to test if that call is executed or not.
I found the bug since I was calling it repeatedly with the same value and that caused some side effects in my application. Or maybe is precisely the invalidate code the one that should be investigated.
I opted to post it as a patch since it seemed the easy way to get it fixed, but now i'm doubtful about it.
http://bugs.winehq.org/show_bug.cgi?id=13112
Vincent Povirk madewokherd@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |madewokherd@gmail.com
--- Comment #6 from Vincent Povirk madewokherd@gmail.com 2008-09-28 20:30:59 --- It sounds to me like the messages are what matter, and you can test for those. There is already code in the tests checking the messages we get. Look for ok_sequence.
And yes, getting changes into Wine tends to be a little bit more work than just writing a patch; Wine is a complicated beast, and we need to be careful that the change is correct. That said, I don't think it will be much more work in this case.
http://bugs.winehq.org/show_bug.cgi?id=13112
--- Comment #7 from Vincent Povirk madewokherd@gmail.com 2008-11-15 21:16:02 --- According to our tests for setpos, Wine is actually missing some WM_PAINT messages, and the whole lot are todo because of that. That makes it impossible to test for the absence of a WM_PAINT on Wine, though we can at least verify the correct behavior.
http://bugs.winehq.org/show_bug.cgi?id=13112
--- Comment #8 from Vincent Povirk madewokherd@gmail.com 2008-11-17 09:39:58 --- This should be fixed in Git.
http://bugs.winehq.org/show_bug.cgi?id=13112
Dmitry Timoshkov dmitry@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED Version|CVS/GIT |unspecified
--- Comment #9 from Dmitry Timoshkov dmitry@codeweavers.com 2008-11-17 10:35:43 --- Should be fixed.
http://bugs.winehq.org/show_bug.cgi?id=13112
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #10 from Alexandre Julliard julliard@winehq.org 2008-11-21 10:31:33 --- Closing bugs fixed in 1.1.9.
-
wine-bugs＠winehq.org