RESOLVED FIXED 199699
Cannot bring up custom media controls at all on v.youku.com
https://bugs.webkit.org/show_bug.cgi?id=199699
Summary Cannot bring up custom media controls at all on v.youku.com
alan
Reported 2019-07-10 19:37:20 PDT
Attachments
Patch (44.79 KB, patch)
2019-07-10 20:13 PDT, alan
no flags
Patch (44.89 KB, patch)
2019-07-10 20:31 PDT, alan
simon.fraser: review-
Patch (48.15 KB, patch)
2019-07-12 09:40 PDT, alan
no flags
Patch (41.89 KB, patch)
2019-07-12 15:20 PDT, alan
no flags
Patch (41.90 KB, patch)
2019-07-12 15:21 PDT, alan
no flags
Patch (41.90 KB, patch)
2019-07-12 20:42 PDT, alan
no flags
alan
Comment 1 2019-07-10 20:13:36 PDT
alan
Comment 2 2019-07-10 20:31:17 PDT
Simon Fraser (smfr)
Comment 3 2019-07-11 11:02:14 PDT
Comment on attachment 373902 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373902&action=review > Source/WebCore/ChangeLog:9 > + The "find the node under the finger" heuristic should only find nodes that are visible to hittesting. hit-testing or hit testing. > Source/WebCore/ChangeLog:13 > + This fixes the case when "visibility: hidden" iFrame covers the target node and it might divert the synthetic click (no hijacking). Please re-write to improve readability. I think you could expand this to explain the two types of hit-testing. > Source/WebCore/page/EventHandler.cpp:1182 > + // hitTestResultAtPointIncludingSubframes is specifically used to hitTest into all frames, thus it always allows child frame content. hitTest -> hit-test > Source/WebCore/page/EventHandler.cpp:1216 > - // hitTestResultAtPoint is specifically used to hitTest into all frames, thus it always allows child frame content. > - HitTestRequest request(hitType | HitTestRequest::AllowChildFrameContent); > + HitTestRequest request(hitType); Should this mask out the AllowChildFrameContent, or assert that it's not set? > Source/WebCore/page/PointerCaptureController.cpp:395 > + target = m_page.mainFrame().eventHandler().hitTestResultAtPointIncludingSubframes(documentPoint, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowChildFrameContent).innerNonSharedElement(); Isn't the HitTestRequest::AllowChildFrameContent redundant here? > Source/WebCore/page/ios/FrameIOS.mm:278 > + auto adjustHitTestResultIfNeeded = [&](auto& hitTestResult) { > + // Retry hittest with the visible-to-hittest restriction on, if the initial hittest finds something inside a non-visible iFrame. > + auto* frame = hitTestResult.innerNodeFrame(); > + if (!frame || frame->isMainFrame()) > + return; > + auto* ownerElement = frame->ownerElement(); > + if (!ownerElement || !ownerElement->renderer()) > + return; > + auto& renderer = *ownerElement->renderer(); > + if (renderer.visibleToHitTesting()) > + return; > + hitTestResult = eventHandler().hitTestResultAtPoint(center); > + }; This seems wrong. You'd have to walk up the entire frame tree looking for any frame ancestor whose renderer is not renderer.visibleToHitTesting(). Also, calling hitTestResultAtPoint(), which doesn't cross frame boundaries, will only check the main frame, but maybe you found something in a nested frame; you'd have to call hitTestResultAtPoint() on that inner frame. I think you need to do the renderer.visibleToHitTesting() check inside of the initial hitTestResultAtPointIncludingSubframes() pass. > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:555 > + HitTestResult result = m_page->mainFrame().eventHandler().hitTestResultAtPointIncludingSubframes(m_lastInteractionLocation, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent); HitTestRequest::AllowChildFrameContent is redundant. > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1494 > + HitTestResult hitTest = frame.eventHandler().hitTestResultAtPointIncludingSubframes(pointInDocument, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent); HitTestRequest::AllowChildFrameContent is redundant. > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2044 > + auto result = m_page->mainFrame().eventHandler().hitTestResultAtPointIncludingSubframes(point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowChildFrameContent); HitTestRequest::AllowChildFrameContent is redundant. > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2653 > + HitTestResult result = page.corePage()->mainFrame().eventHandler().hitTestResultAtPointIncludingSubframes(request.point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowChildFrameContent); HitTestRequest::AllowChildFrameContent is redundant. > Source/WebKitLegacy/ios/WebCoreSupport/WebFrameIOS.mm:944 > + HitTestResult result = frame->eventHandler().hitTestResultAtPointIncludingSubframes(adjustedPoint, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent); HitTestRequest::AllowChildFrameContent is redundant.
alan
Comment 4 2019-07-11 11:32:36 PDT
> This seems wrong. You'd have to walk up the entire frame tree looking for > any frame ancestor whose renderer is not renderer.visibleToHitTesting(). > Also, calling hitTestResultAtPoint(), which doesn't cross frame boundaries, > will only check the main frame, but maybe you found something in a nested > frame; you'd have to call hitTestResultAtPoint() on that inner frame. I think what I need to do is to look at the HitTestResult::isOverWidget flag and extract the target subframe. It's exactly what we do during mouse event dispatching (start the event with the mainframe and look inside the HitTestResult to find the target subframe).
alan
Comment 5 2019-07-12 09:40:02 PDT
alan
Comment 6 2019-07-12 15:20:01 PDT
alan
Comment 7 2019-07-12 15:21:57 PDT
alan
Comment 8 2019-07-12 20:42:47 PDT
WebKit Commit Bot
Comment 9 2019-07-13 05:59:42 PDT
Comment on attachment 374076 [details] Patch Clearing flags on attachment: 374076 Committed r247416: <https://trac.webkit.org/changeset/247416>
WebKit Commit Bot
Comment 10 2019-07-13 05:59:43 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.