Re: comctl32/listview: update dwStyle before checking for scrollbar
On 9/21/2012 23:07, Daniel Jelinski wrote:
dwStyle is updated only in WM_STYLECHANGED, which is not sent when displaying and hiding scrollbars. Fixes the issue where there was a large margin between items and horizontal scrollbar (e.g. in file open dialog). Are you sure it's really not supposed to be sent when WS_HSCROLL/WS_VSCROLL bits are changed? A quick test with ControlSpy shows style changing messages when I set/reset these. So it's possible there's a bug outside of comctl32.
Hello Nikolay, I'm not sure. There are some tests: http://source.winehq.org/source/dlls/user32/tests/msg.c#L1613 Comments indicate that some systems send WM_STYLECHANGED to windows with non-client area, and no systems send it to windows without it. I haven't figured out yet if the comment is true or to which category ListView belongs. Out of curiosity I checked which systems send WM_STYLECHANGED: http://testbot.winehq.org/JobDetails.pl?Key=21701 Vista and Windows 7 do send it, others do not, neither does Wine. Judging by the code it doesn't look like a quick fix to me (but I wouldn't mind if someone proved me wrong here). Regards, Daniel PS. I guess I should have used GetWindowLong instead on GetWindowLongPtr. Not sure if I should bother with resending now. 2012/9/22 Nikolay Sivov <nsivov(a)codeweavers.com>:
On 9/21/2012 23:07, Daniel Jelinski wrote:
dwStyle is updated only in WM_STYLECHANGED, which is not sent when displaying and hiding scrollbars. Fixes the issue where there was a large margin between items and horizontal scrollbar (e.g. in file open dialog).
Are you sure it's really not supposed to be sent when WS_HSCROLL/WS_VSCROLL bits are changed? A quick test with ControlSpy shows style changing messages when I set/reset these. So it's possible there's a bug outside of comctl32.
On 9/22/2012 20:41, Daniel Jelinski wrote:
Hello Nikolay, I'm not sure. There are some tests: http://source.winehq.org/source/dlls/user32/tests/msg.c#L1613 Comments indicate that some systems send WM_STYLECHANGED to windows with non-client area, and no systems send it to windows without it. I haven't figured out yet if the comment is true or to which category ListView belongs.
Out of curiosity I checked which systems send WM_STYLECHANGED: http://testbot.winehq.org/JobDetails.pl?Key=21701 Vista and Windows 7 do send it, others do not, neither does Wine. Judging by the code it doesn't look like a quick fix to me (but I wouldn't mind if someone proved me wrong here). I see. Now a question is how scroll bars are enabled/disabled in a case you're trying to fix, and a message test should be added for listview after that. What I'm seeing - ControlSpy running on XP shows style change messages after I toggle scrollbar bits, but we don't have its source so who knows what it does.
And apparently we have code in WM_STYLECHANGED handler to deal with scroll bars: --- if (((lpss->styleOld & WS_HSCROLL) != 0)&& ((lpss->styleNew & WS_HSCROLL) == 0)) ShowScrollBar(infoPtr->hwndSelf, SB_HORZ, FALSE); if (((lpss->styleOld & WS_VSCROLL) != 0)&& ((lpss->styleNew & WS_VSCROLL) == 0)) ShowScrollBar(infoPtr->hwndSelf, SB_VERT, FALSE); --- these lines were edited 10 years ago last time, sure it could be a noop but that raises a question why it's here.
Regards, Daniel
PS. I guess I should have used GetWindowLong instead on GetWindowLongPtr. Not sure if I should bother with resending now.
It doesn't matter in case of retrieving style bits, server stores it as unsigned int, so most significant int in 64-bits is always zeroed I guess.
2012/9/22 Nikolay Sivov <nsivov(a)codeweavers.com>:
On 9/21/2012 23:07, Daniel Jelinski wrote:
dwStyle is updated only in WM_STYLECHANGED, which is not sent when displaying and hiding scrollbars. Fixes the issue where there was a large margin between items and horizontal scrollbar (e.g. in file open dialog). Are you sure it's really not supposed to be sent when WS_HSCROLL/WS_VSCROLL bits are changed? A quick test with ControlSpy shows style changing messages when I set/reset these. So it's possible there's a bug outside of comctl32.
2012/9/22 Nikolay Sivov <nsivov(a)codeweavers.com>:
I see. Now a question is how scroll bars are enabled/disabled in a case you're trying to fix, and a message test should be added for listview after that. What I'm seeing - ControlSpy running on XP shows style change messages after I toggle scrollbar bits, but we don't have its source so who knows what it does.
Do you toggle these bits using SetWindowLong or SetScrollInfo? Wine uses SetScrollInfo, which is probably the reason why we don't get these messages. My first try at this bug was updating cached dwStyle in LISTVIEW_UpdateScroll: http://source.winehq.org/source/dlls/comctl32/listview.c#L2058 However, that was not sufficient for a sample application I created in Delphi - seems that Delphi creates the scrollbar elsewhere. Probably the code could be rewritten to not depend on WS_HSCROLL and WS_VSRCOLL being correct: http://source.winehq.org/source/dlls/comctl32/listview.c#L10881 is the only line that depends on it.
And apparently we have code in WM_STYLECHANGED handler to deal with scroll bars:
--- if (((lpss->styleOld & WS_HSCROLL) != 0)&& ((lpss->styleNew & WS_HSCROLL) == 0)) ShowScrollBar(infoPtr->hwndSelf, SB_HORZ, FALSE);
if (((lpss->styleOld & WS_VSCROLL) != 0)&& ((lpss->styleNew & WS_VSCROLL) == 0)) ShowScrollBar(infoPtr->hwndSelf, SB_VERT, FALSE); ---
these lines were edited 10 years ago last time, sure it could be a noop but that raises a question why it's here.
Dead code. I meant to remove these in the next patch, since there are no bugs against scrollbars in listview. I guess user32 was way less capable back then. Regards, Daniel
2012/9/22 Daniel Jelinski <djelinski1(a)gmail.com>:
2012/9/22 Nikolay Sivov <nsivov(a)codeweavers.com>:
And apparently we have code in WM_STYLECHANGED handler to deal with scroll bars:
--- if (((lpss->styleOld & WS_HSCROLL) != 0)&& ((lpss->styleNew & WS_HSCROLL) == 0)) ShowScrollBar(infoPtr->hwndSelf, SB_HORZ, FALSE);
if (((lpss->styleOld & WS_VSCROLL) != 0)&& ((lpss->styleNew & WS_VSCROLL) == 0)) ShowScrollBar(infoPtr->hwndSelf, SB_VERT, FALSE); ---
these lines were edited 10 years ago last time, sure it could be a noop but that raises a question why it's here.
Dead code. I meant to remove these in the next patch, since there are no bugs against scrollbars in listview. I guess user32 was way less capable back then.
Now that I think about it, it's not exactly dead code - it will be executed when someone tries to hide scrollbars by setting window style. Needs a more thorough review. Regards, Daniel
participants (2)
-
Daniel Jelinski -
Nikolay Sivov