RESOLVED FIXED 134684
ServicesOverlayController menus show up in the wrong place
https://bugs.webkit.org/show_bug.cgi?id=134684
Summary ServicesOverlayController menus show up in the wrong place
Brady Eidson
Reported 2014-07-07 10:21:12 PDT
ServicesOverlayController menus show up in the wrong place. This is despite the fact that they use the context menu mechanism which does display context menus in the right place. The difference is that context menu clicks undergo a hit test and have their coordinate translated to window-space. We need to do the same for Services. <rdar://problem/17130576>
Attachments
Patch v1 (3.51 KB, patch)
2014-07-07 10:25 PDT, Brady Eidson
no flags
Patch v2 (3.41 KB, patch)
2014-07-07 11:40 PDT, Brady Eidson
thorton: review+
Brady Eidson
Comment 1 2014-07-07 10:25:25 PDT
Created attachment 234496 [details] Patch v1
Tim Horton
Comment 2 2014-07-07 11:04:36 PDT
Comment on attachment 234496 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=234496&action=review > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:434 > +void ServicesOverlayController::handleClick(const WebCore::IntPoint& clickPoint) This seems wrong. The incoming point is always in a consistent coordinate space (though I'm not sure at the moment which one), not dependent on what frame you click on. Am I wrong and this somehow actually works in subframes?
Brady Eidson
Comment 3 2014-07-07 11:31:46 PDT
Discussed with Tim in person - This patch wasn't *wrong*, it was just more work than was needed. Simpler patch coming up, that also includes a fix for a subframe crasher that showed up in testing subframes.
Brady Eidson
Comment 4 2014-07-07 11:40:09 PDT
Created attachment 234499 [details] Patch v2 Confirmed with various main frame scroll positions, subframe scroll positions, and page zoom situations, that this patch gives the same coordinates as the hit test patch.
Tim Horton
Comment 5 2014-07-07 11:41:50 PDT
Comment on attachment 234499 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=234499&action=review > Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:435 > + Page* page = m_webPage->corePage(); Please use WebPage::mainFrameView() instead of all of this.
Brady Eidson
Comment 6 2014-07-07 11:51:28 PDT
WebKit Commit Bot
Comment 7 2014-07-09 16:57:27 PDT
Re-opened since this is blocked by bug 134783
Brady Eidson
Comment 8 2014-07-09 17:09:44 PDT
(In reply to comment #7) > Re-opened since this is blocked by bug 134783 That was just a test of webkitbot. All is well.
Note You need to log in before you can comment on or make changes to this bug.