The existing code determines which screen the popup menu "belongs" to by determining which screen the owning application's hwnd belongs to, where ownership is determined by how much of the hwnd is on which screen. To match what most Windows applications do, the owning screen should be whichever screen the drop down button belongs to. To determine which screen an element belongs to in Windows we need to pass in an hwnd for that element. However, since the drop down button is something that WebKit renders there is no hwnd. To remedy this issue, we can temporarily move the popup menu's hwnd to match the position and size of the button, determine the correct screen, and then eventually move it back to the correct final position after the rest of the calculations have been completed. This is all done in the same function call so no rendering of the popup menu occurs between the temporary and final positionings.
Created attachment 170533 [details] patch
<rdar://problem/12570435>
Comment on attachment 170533 [details] patch Attachment 170533 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14593039
Comment on attachment 170533 [details] patch Attachment 170533 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14553454
Comment on attachment 170533 [details] patch Attachment 170533 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14554464
Comment on attachment 170533 [details] patch Attachment 170533 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14552436
Comment on attachment 170533 [details] patch Attachment 170533 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14566416
Comment on attachment 170533 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=170533&action=review > Source/WebCore/platform/PlatformScreen.h:55 > + FloatRect monitorFromHwnd(HWND); This will certainly break all the builds except windows, since HWND is a windows type. I'm also not clear on why this should be in platform code, if it is only called from one place.
(In reply to comment #8) > (From update of attachment 170533 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=170533&action=review > > > Source/WebCore/platform/PlatformScreen.h:55 > > + FloatRect monitorFromHwnd(HWND); > > This will certainly break all the builds except windows, since HWND is a windows type. Right... > I'm also not clear on why this should be in platform code, if it is only called from one place. I stuck it there because it looked like many similar methods were coming from there (such as getting the screen from a Widget). Looking at PlatformScreenWin.cpp again, perhaps it makes more sense to just to make the method static and stick it in there. That seems to be what we did for other Windows specific screen info getting methods. Does that seem reasonable or should I just stick it in PopupMenuWin?
Err right, I can't do that of course...static in a source file means no access to anything but source file...right... Anyways, I'll just stick it with PopupMenuWin.h and call it a day.
Created attachment 170749 [details] revised patch
Comment on attachment 170749 [details] revised patch View in context: https://bugs.webkit.org/attachment.cgi?id=170749&action=review > Source/WebCore/platform/win/PopupMenuWin.cpp:694 > +FloatRect PopupMenuWin::monitorFromHwnd(HWND hwnd) > +{ > + HMONITOR monitor = MonitorFromWindow(hwnd, MONITOR_DEFAULTTOPRIMARY); > + MONITORINFOEX monitorInfo; > + monitorInfo.cbSize = sizeof(MONITORINFOEX); > + GetMonitorInfo(monitor, &monitorInfo); > + return monitorInfo.rcWork; > +} This can just be a static function, no need to be a member function.
Created attachment 171501 [details] made monitorFromHwnd static
Committed here: http://trac.webkit.org/changeset/133092