Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(672)

Issue 137403008: Don't set the scroll styles (WS_VSCROLL and WS_HSCROLL) for WS_POPUP windows. (Closed)

Created:
6 years, 11 months ago by ananta
Modified:
6 years, 11 months ago
Reviewers:
sky
CC:
chromium-reviews, tfarina
Visibility:
Public.

Description

Don't set the scroll styles (WS_VSCROLL and WS_HSCROLL) for WS_POPUP windows. This causes issues with select boxes on Windows 7 where hovering at the end of the window returns the scroll WM_NCHITTEST codes. Works fine on Windows 8. In any case we don't want the scrolling styles set on windows other than the main Chrome window which is the only window which should be receive mousewheel messages. I moved the scroll style setting code from the HWNDMessageHandler::OnCreate function to HWNDMessageHandler::Init function as that would prevent the initial WM_SIZE message from hiding the scrollbar. The other change is to hide the scrollbars and readd the scroll styles if we are sizing the window, when the sizing completes. Basically when we receive the WM_EXITSIZEMOVE message. For normal sizing operations we continue to do this in WM_SIZE as before. From testing on my thinkpad with Win7, desktop with Win7 and Win8 this works well. BUG=334454, 334541 R=sky@chromium.org, sky Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245051

Patch Set 1 : #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 6

Patch Set 4 : #

Total comments: 2

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -30 lines) Patch
M ui/views/win/hwnd_message_handler.h View 1 2 3 4 1 chunk +5 lines, -2 lines 0 comments Download
M ui/views/win/hwnd_message_handler.cc View 1 2 3 4 6 chunks +46 lines, -28 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
ananta
6 years, 11 months ago (2014-01-15 02:58:27 UTC) #1
sky
LGTM
6 years, 11 months ago (2014-01-15 14:21:45 UTC) #2
ananta
PTAL. I updated the patch to remove the scroll styles and readd them at the ...
6 years, 11 months ago (2014-01-15 15:38:28 UTC) #3
sky
https://codereview.chromium.org/137403008/diff/330001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/137403008/diff/330001/ui/views/win/hwnd_message_handler.cc#newcode320 ui/views/win/hwnd_message_handler.cc:320: base::MessageLoop::current()->PostTask( Document why the post task. https://codereview.chromium.org/137403008/diff/330001/ui/views/win/hwnd_message_handler.cc#newcode451 ui/views/win/hwnd_message_handler.cc:451: ::SetWindowLong(hwnd(), ...
6 years, 11 months ago (2014-01-15 15:41:32 UTC) #4
ananta
https://codereview.chromium.org/137403008/diff/330001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/137403008/diff/330001/ui/views/win/hwnd_message_handler.cc#newcode320 ui/views/win/hwnd_message_handler.cc:320: base::MessageLoop::current()->PostTask( On 2014/01/15 15:41:32, sky wrote: > Document why ...
6 years, 11 months ago (2014-01-15 16:54:58 UTC) #5
sky
LGTM - can you try running a nested message loop and make sure there is ...
6 years, 11 months ago (2014-01-15 17:29:00 UTC) #6
ananta
https://codereview.chromium.org/137403008/diff/390001/ui/views/win/hwnd_message_handler.h File ui/views/win/hwnd_message_handler.h (right): https://codereview.chromium.org/137403008/diff/390001/ui/views/win/hwnd_message_handler.h#newcode546 ui/views/win/hwnd_message_handler.h:546: bool size_move_loop_; On 2014/01/15 17:29:01, sky wrote: > nit: ...
6 years, 11 months ago (2014-01-15 20:10:16 UTC) #7
ananta
6 years, 11 months ago (2014-01-16 01:45:24 UTC) #8
Message was sent while issue was closed.
Committed patchset #5 manually as r245051 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698