RESOLVED FIXED 147880
[Win] Popup menus have incorrect placement when device scale factor != 1.
https://bugs.webkit.org/show_bug.cgi?id=147880
Summary [Win] Popup menus have incorrect placement when device scale factor != 1.
peavo
Reported 2015-08-11 07:33:24 PDT
We need to take the device scaling factor into account when calculating the position and size of the popup menu.
Attachments
Patch (2.31 KB, patch)
2015-08-11 07:37 PDT, peavo
no flags
peavo
Comment 1 2015-08-11 07:37:53 PDT
Brent Fulgham
Comment 2 2015-08-11 09:26:49 PDT
Comment on attachment 258717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258717&action=review r=me. > Source/WebCore/platform/win/PopupMenuWin.cpp:291 > + if (!::ClientToScreen(hostWindow, &absoluteLocation)) Heh -- nice simplification! :-) > Source/WebCore/platform/win/PopupMenuWin.cpp:317 > + Page* page = v->frame().page(); How about: "if (Page* page = v->frame().page()) ..." ?
peavo
Comment 3 2015-08-11 09:40:41 PDT
(In reply to comment #2) > Comment on attachment 258717 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=258717&action=review > > r=me. > Thanks! > > Source/WebCore/platform/win/PopupMenuWin.cpp:291 > > + if (!::ClientToScreen(hostWindow, &absoluteLocation)) > > Heh -- nice simplification! :-) > > > Source/WebCore/platform/win/PopupMenuWin.cpp:317 > > + Page* page = v->frame().page(); > > How about: "if (Page* page = v->frame().page()) ..." ? I would have changed this, but now I see it's already in the commit queue :)
Brent Fulgham
Comment 4 2015-08-11 09:42:04 PDT
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 258717 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=258717&action=review > > > > r=me. > > > > Thanks! > > > > Source/WebCore/platform/win/PopupMenuWin.cpp:291 > > > + if (!::ClientToScreen(hostWindow, &absoluteLocation)) > > > > Heh -- nice simplification! :-) > > > > > Source/WebCore/platform/win/PopupMenuWin.cpp:317 > > > + Page* page = v->frame().page(); > > > > How about: "if (Page* page = v->frame().page()) ..." ? > > I would have changed this, but now I see it's already in the commit queue :) Yes -- that's my fault! Don't worry about the change. I was just writing comments as I read the patch. :-)
peavo
Comment 5 2015-08-11 09:59:26 PDT
(In reply to comment #4) > > Yes -- that's my fault! Don't worry about the change. I was just writing > comments as I read the patch. :-) Ok, thanks :)
WebKit Commit Bot
Comment 6 2015-08-11 10:16:12 PDT
Comment on attachment 258717 [details] Patch Clearing flags on attachment: 258717 Committed r188259: <http://trac.webkit.org/changeset/188259>
WebKit Commit Bot
Comment 7 2015-08-11 10:16:15 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.