WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
<
rdar://problem/51835327
>
Attachments
Patch
(44.79 KB, patch)
2019-07-10 20:13 PDT
,
alan
no flags
Details
Formatted Diff
Diff
Patch
(44.89 KB, patch)
2019-07-10 20:31 PDT
,
alan
simon.fraser
: review-
Details
Formatted Diff
Diff
Patch
(48.15 KB, patch)
2019-07-12 09:40 PDT
,
alan
no flags
Details
Formatted Diff
Diff
Patch
(41.89 KB, patch)
2019-07-12 15:20 PDT
,
alan
no flags
Details
Formatted Diff
Diff
Patch
(41.90 KB, patch)
2019-07-12 15:21 PDT
,
alan
no flags
Details
Formatted Diff
Diff
Patch
(41.90 KB, patch)
2019-07-12 20:42 PDT
,
alan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
alan
Comment 1
2019-07-10 20:13:36 PDT
Created
attachment 373899
[details]
Patch
alan
Comment 2
2019-07-10 20:31:17 PDT
Created
attachment 373902
[details]
Patch
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
Created
attachment 374016
[details]
Patch
alan
Comment 6
2019-07-12 15:20:01 PDT
Created
attachment 374042
[details]
Patch
alan
Comment 7
2019-07-12 15:21:57 PDT
Created
attachment 374043
[details]
Patch
alan
Comment 8
2019-07-12 20:42:47 PDT
Created
attachment 374076
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug