Created attachment 34619 [details] Fix Mouse wheel scrolling on a page with expanded drop down list detaches drop down <rdar://6728361>
Comment on attachment 34619 [details] Fix While it looks like this fixes the bug when the mousewheel is used, what happens if a page triggers a scroll on a timer? e.g., the page sets a timer for 5 seconds in the future to scroll the page, the user opens the popup menu, then the timer fires?
Other browser behavior: Opera - Passes wheel events to pop up menu. IE - Ignores Wheel Event. Firefox - Closes popup window, doesn't scroll window. Chrome - Closes popup window, scrolls window. Safari for Mac - Ignores Wheel Event.
Closing the window seems like fine behavior. It would probably be hard to get the popup window to move in sync with the scrolling of the WebView anyway.
Created attachment 34691 [details] Little Cleaner Way
Comment on attachment 34691 [details] Little Cleaner Way > From 2326797b9b9af58a1e28949e516be86d21e20a3a Mon Sep 17 00:00:00 2001 > From: Brian Weinstein <bweinstein@apple.com> > Date: Wed, 12 Aug 2009 14:33:44 -0700 > Subject: [PATCH] 2009-08-12 Brian Weinstein <bweinstein@apple.com> > > Reviewed by NOBODY (OOPS!). > > Fix of <rdar://6728361> Mouse wheel scrolling on a page with expanded drop down > list detaches drop down. > > Added a check in mouseWheel to see if our focus is currently in a popup, if so, close > the popup (matches other browser behavior). > > * WebView.cpp: > (WebView::mouseWheel): > > 2009-08-12 Brian Weinstein <bweinstein@apple.com> > > Reviewed by NOBODY (OOPS!). > > Fix of <rdar://6728361> Mouse wheel scrolling on a page with expanded drop down > list detaches drop down. > > Added a function for Windows PopupMenu's to return their class name. > > * platform/PopupMenu.h: > * platform/win/PopupMenuWin.cpp: > (WebCore::PopupMenu::popupClassName): Looks like your commit message got doubled. > + HWND focusHwnd; > + if ((focusHwnd = GetFocus()) && focusHwnd != m_viewWindow) { I'd move the GetFocus() call up to the previous line. HWND focusHwnd = GetFocus(); if (focusHwnd && focusHwnd != m_viewWindow) { "focusedWindow" might be nicer than "focusHwnd". > + // Our focus is on a different hwnd, see if it's a PopupMenu and if so, set the focus back on us (which will hide the popup). > + TCHAR className[256]; > + if (GetClassName(focusHwnd, className, ARRAYSIZE(className)) && !_tcscmp(className, PopupMenu::popupClassName())) { > + SetFocus(m_viewWindow); > + return true; > + } > + } It would be good to add an assert like ASSERT(ARRAYSIZE(className) > _tcslen(PopupMenu::popupClassName()) so that we know the possible truncation of the class name won't affect the comparison. You should add a FIXME mentioning the other scrolling-related popup menu bug and how we'd like to fix them both together. You should add a comment about why we don't let the WebView scroll in response to this event. r=me
Committed in http://trac.webkit.org/changeset/47160.