Bug 134684 – ServicesOverlayController menus show up in the wrong place

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

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>

[Brady Eidson]

Created attachment 234496 [details]
Patch v1

[Tim Horton]

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]

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]

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 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]

http://trac.webkit.org/changeset/170852

[WebKit Commit Bot]

Re-opened since this is blocked by bug 134783

[Brady Eidson]

(In reply to comment #7)
> Re-opened since this is blocked by bug 134783

That was just a test of webkitbot.  All is well.