Add top-document and iframe-document resource types to WKContentRuleList syntax
Created attachment 414271 [details] Patch
<rdar://problem/71747736>
Created attachment 416645 [details] Patch
I think we can fulfill the original request while making something more powerful. Instead of creating a new resource type, add a new field that specify the context: All, MainFrame, Iframe, SubFrame. Something like that. The whole rule would be ignored if not in the right context. This would allow any rule to be applied specifically based on the context. I assume it is common to block things when in frames but not in the top document (say tracking cookies from Facebook for example). The downside is more useless matching if we filter a posteriori. If that becomes an issue, it can be fixed by splitting the filter by context a priori. What do you think?
Sounds like something to discuss next year. That might be necessary to fix rdar://problem/72203352 too.
That would not allow us the same functionality as having a nested-frame resource type. An iframe's main resource is loaded in the context of its parent frame, so we would be unable to specify that we would like to block iframe main resources with your suggestion.
Created attachment 424175 [details] Patch
Comment on attachment 424175 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424175&action=review > Source/WebCore/loader/ResourceLoadInfo.cpp:132 > + if (name == "iframe") > + return { LoadContext::IFrame }; I think 'iframe' is the wrong name here because 'iframe' implies only <iframe> elements, but child frames can also be <frame>, <object>, or <embed>. Here's an alternative that matches web speak a little more: { top-frame, child-frame } Here's an alternative that matches our parlance a little more: { main-frame, sub-frame } I'd be happy with either of those, or something else that avoids 'iframe'. I don't think we should do 'iframe'.
Created attachment 424204 [details] Patch
Comment on attachment 424204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424204&action=review > Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:271 > + ResourceLoadInfo resourceLoadInfo = { url, mainDocumentURL, { ResourceType::Ping, ResourceType::Other } }; Why not add ping to the "other" resource type in readResourceType? Wouldn't this be more consistent with the other loaded resources? > Source/WebCore/loader/PingLoader.cpp:125 > + if (processContentRuleListsForLoad(frame, request, { ContentExtensions::ResourceType::Other, ContentExtensions::ResourceType::Ping })) Ditto here regarding ping flags. > Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentRuleListNotification.mm:307 > + auto handler = [[TestURLSchemeHandler new] autorelease]; I hope TEST has an autorelease pool :-D > Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentRuleListNotification.mm:373 > + [userContentController addContentRuleList:listWithLoadContext("child-frame").get()]; > + [webView reload]; > + EXPECT_WK_STREQ([webView _test_waitForAlert], "main frame fetched successfully"); > + EXPECT_WK_STREQ([webView _test_waitForAlert], "iframe fetch failed"); > + > + [userContentController removeAllContentRuleLists]; > + [userContentController addContentRuleList:listWithLoadContext("top-frame").get()]; > + [webView reload]; > + EXPECT_WK_STREQ([webView _test_waitForAlert], "main frame fetch failed"); > + EXPECT_WK_STREQ([webView _test_waitForAlert], "iframe fetched successfully"); I like those tests!
Created attachment 424314 [details] Patch
Ben Poulain found in /Volumes/Data/worker/Commit-Queue/build/Source/WebCore/ChangeLog does not appear to be a valid reviewer according to contributors.json. /Volumes/Data/worker/Commit-Queue/build/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Update looks good to me! (In reply to EWS from comment #12) > Ben Poulain found in > /Volumes/Data/worker/Commit-Queue/build/Source/WebCore/ChangeLog does not > appear to be a valid reviewer according to contributors.json. > /Volumes/Data/worker/Commit-Queue/build/Source/WebCore/ChangeLog neither > lists a valid reviewer nor contains the string "Unreviewed" or "Rubber > stamp" (case insensitive). You may need to use my full name.
r275078
*** Bug 153559 has been marked as a duplicate of this bug. ***