147880 2015-08-11 07:33:24 -0700 [Win] Popup menus have incorrect placement when device scale factor != 1. 2015-08-11 10:16:15 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 peavo webkit-unassigned achristensen bfulgham commit-queue oldest_to_newest 1116531 0 peavo 2015-08-11 07:33:24 -0700 We need to take the device scaling factor into account when calculating the position and size of the popup menu. 1116534 1 258717 peavo 2015-08-11 07:37:53 -0700 Created attachment 258717 Patch 1116548 2 258717 bfulgham 2015-08-11 09:26:49 -0700 Comment on attachment 258717 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()) ..." ? 1116549 3 peavo 2015-08-11 09:40:41 -0700 (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 :) 1116550 4 bfulgham 2015-08-11 09:42:04 -0700 (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. :-) 1116563 5 peavo 2015-08-11 09:59:26 -0700 (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 :) 1116566 6 258717 commit-queue 2015-08-11 10:16:12 -0700 Comment on attachment 258717 Patch Clearing flags on attachment: 258717 Committed r188259: <http://trac.webkit.org/changeset/188259> 1116567 7 commit-queue 2015-08-11 10:16:15 -0700 All reviewed patches have been landed. Closing bug. 258717 2015-08-11 07:37:53 -0700 2015-08-11 10:16:12 -0700 Patch bug-147880-20150811163527.patch text/plain 2364 peavo SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4ODI1MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDE1LTA4LTExICBQZXIgQXJu ZSBWb2xsYW4gIDxwZWF2b0BvdXRsb29rLmNvbT4KKworICAgICAgICBbV2luXSBQb3B1cCBtZW51 cyBoYXZlIGluY29ycmVjdCBwbGFjZW1lbnQgd2hlbiBkZXZpY2Ugc2NhbGUgZmFjdG9yICE9IDEu CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDc4ODAK KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXZSBuZWVk IHRvIHRha2UgdGhlIGRldmljZSBzY2FsaW5nIGZhY3RvciBpbnRvIGFjY291bnQgd2hlbiBjYWxj dWxhdGluZworICAgICAgICB0aGUgcG9zaXRpb24gYW5kIHNpemUgb2YgdGhlIHBvcHVwIG1lbnUu CisKKyAgICAgICAgKiBwbGF0Zm9ybS93aW4vUG9wdXBNZW51V2luLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OlBvcHVwTWVudVdpbjo6Y2FsY3VsYXRlUG9zaXRpb25BbmRTaXplKToKKwogMjAxNS0w OC0xMCAgQ2hyaXMgRHVtZXogIDxjZHVtZXpAYXBwbGUuY29tPgogCiAgICAgICAgIFRoZSAncHJv dG90eXBlJyBwcm9wZXJ0eSBvbiBpbnRlcmZhY2Ugb2JqZWN0cyBzaG91bGQgbm90IGJlIGVudW1l cmFibGUKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3dpbi9Qb3B1cE1lbnVXaW4uY3Bw Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3dpbi9Qb3B1cE1lbnVXaW4u Y3BwCShyZXZpc2lvbiAxODgyMTQpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS93aW4vUG9w dXBNZW51V2luLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjg4LDcgKzI4OCw3IEBAIHZvaWQgUG9w dXBNZW51V2luOjpjYWxjdWxhdGVQb3NpdGlvbkFuZFMKICAgICBJbnRSZWN0IGFic29sdXRlQm91 bmRzID0gKChSZW5kZXJNZW51TGlzdCopbV9wb3B1cENsaWVudCktPmFic29sdXRlQm91bmRpbmdC b3hSZWN0KCk7CiAgICAgSW50UmVjdCBhYnNvbHV0ZVNjcmVlbkNvb3Jkcyh2LT5jb250ZW50c1Rv V2luZG93KGFic29sdXRlQm91bmRzLmxvY2F0aW9uKCkpLCBhYnNvbHV0ZUJvdW5kcy5zaXplKCkp OwogICAgIFBPSU5UIGFic29sdXRlTG9jYXRpb24oYWJzb2x1dGVTY3JlZW5Db29yZHMubG9jYXRp b24oKSk7Ci0gICAgaWYgKCE6OkNsaWVudFRvU2NyZWVuKHYtPmhvc3RXaW5kb3coKS0+cGxhdGZv cm1QYWdlQ2xpZW50KCksICZhYnNvbHV0ZUxvY2F0aW9uKSkKKyAgICBpZiAoITo6Q2xpZW50VG9T Y3JlZW4oaG9zdFdpbmRvdywgJmFic29sdXRlTG9jYXRpb24pKQogICAgICAgICByZXR1cm47CiAg ICAgYWJzb2x1dGVTY3JlZW5Db29yZHMuc2V0TG9jYXRpb24oYWJzb2x1dGVMb2NhdGlvbik7CiAK QEAgLTMxMywxMSArMzEzLDE1IEBAIHZvaWQgUG9wdXBNZW51V2luOjpjYWxjdWxhdGVQb3NpdGlv bkFuZFMKICAgICAvLyByIGlzIGluIGFic29sdXRlIGRvY3VtZW50IGNvb3JkaW5hdGVzLCBidXQg d2Ugd2FudCB0byBiZSBpbiBzY3JlZW4gY29vcmRpbmF0ZXMuCiAKICAgICAvLyBGaXJzdCwgbW92 ZSB0byBXZWJWaWV3IGNvb3JkaW5hdGVzCi0gICAgSW50UmVjdCByU2NyZWVuQ29vcmRzKHYtPmNv bnRlbnRzVG9XaW5kb3coci5sb2NhdGlvbigpKSwgci5zaXplKCkpOworICAgIEludFJlY3QgclNj YWxlZCA9IHI7CisgICAgUGFnZSogcGFnZSA9IHYtPmZyYW1lKCkucGFnZSgpOworICAgIGlmIChw YWdlKQorICAgICAgICByU2NhbGVkLnNjYWxlKHBhZ2UtPmRldmljZVNjYWxlRmFjdG9yKCkpOwor ICAgIEludFJlY3QgclNjcmVlbkNvb3Jkcyh2LT5jb250ZW50c1RvV2luZG93KHJTY2FsZWQubG9j YXRpb24oKSksIHJTY2FsZWQuc2l6ZSgpKTsKIAogICAgIC8vIFRoZW4sIHRyYW5zbGF0ZSB0byBz Y3JlZW4gY29vcmRpbmF0ZXMKICAgICBQT0lOVCBsb2NhdGlvbihyU2NyZWVuQ29vcmRzLmxvY2F0 aW9uKCkpOwotICAgIGlmICghOjpDbGllbnRUb1NjcmVlbih2LT5ob3N0V2luZG93KCktPnBsYXRm b3JtUGFnZUNsaWVudCgpLCAmbG9jYXRpb24pKQorICAgIGlmICghOjpDbGllbnRUb1NjcmVlbiho b3N0V2luZG93LCAmbG9jYXRpb24pKQogICAgICAgICByZXR1cm47CiAKICAgICByU2NyZWVuQ29v cmRzLnNldExvY2F0aW9uKGxvY2F0aW9uKTsK