Summary: | Change PopupMenu positioning on Windows such that behaviour on multiple monitors matches Windows standards. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Roger Fong <roger_fong> | ||||||||
Component: | Forms | Assignee: | Roger Fong <roger_fong> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dglazkov, gtk-ews, gustavo, mitz, peter+ews, roger_fong, webkit-bug-importer, xan.lopez | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows 7 | ||||||||||
Attachments: |
|
Description
Roger Fong
2012-10-24 18:26:03 PDT
Created attachment 170533 [details]
patch
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 |