Bug 140584

Summary: Add initial experimental user content filtering API
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, japhet
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch benjamin: review+

Description Sam Weinig 2015-01-17 16:07:24 PST
Add initial experimental user content filtering API
Comment 1 Sam Weinig 2015-01-17 16:20:23 PST
Created attachment 244844 [details]
Patch
Comment 2 Sam Weinig 2015-01-17 16:25:21 PST
The one big thing I think this API is missing, and that will need to be addressed, is dealing with a compilation phase of the rule list. This is largely due to the fact that I am not sure what we want our model to be. Should all loads be deferred when we are compiling the rule list? Should the client of the API have to explicitly ask for the rule list to be compiled and then pass the compiled list to WKUserContentController?
Comment 3 Sam Weinig 2015-01-17 21:11:05 PST
Created attachment 244852 [details]
Patch
Comment 4 Benjamin Poulain 2015-01-18 12:44:59 PST
Comment on attachment 244852 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244852&action=review

Thanks a lot for building the APIs!

General comments:
-IMHO, rulelist is a poor name for the JSON string.
-You really like making interfaces :)

> Source/WebKit2/UIProcess/API/Cocoa/WKUserContentControllerPrivate.h:35
> +- (void)_removeAllUserContentFilters;

I am curious why you added remove-all but not remove-one.

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentFilter.h:31
> +@interface _WKUserContentFilter : NSObject

I am not a fan of the name ContentFilter. Hopefully we will do more than filtering content :)

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentFilter.h:33
> +- (instancetype)_initWithName:(NSString *)name ruleList:(NSString *)ruleList;

Isn't it a little weird to call the argument ruleList since it is a string?
Comment 5 Sam Weinig 2015-01-18 13:42:21 PST
(In reply to comment #4)
> Comment on attachment 244852 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244852&action=review
> 
> Thanks a lot for building the APIs!
> 
> General comments:
> -IMHO, rulelist is a poor name for the JSON string.

I'll use serializedRules as you did.

> -You really like making interfaces :)

:)

> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKUserContentControllerPrivate.h:35
> > +- (void)_removeAllUserContentFilters;
> 
> I am curious why you added remove-all but not remove-one.

Just to match UserScripts, and no other good reason.

> 
> > Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentFilter.h:31
> > +@interface _WKUserContentFilter : NSObject
> 
> I am not a fan of the name ContentFilter. Hopefully we will do more than
> filtering content :)

Fair enough.  I thought ContentExtension was too broad :). We will have to find something in the middle.

> 
> > Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentFilter.h:33
> > +- (instancetype)_initWithName:(NSString *)name ruleList:(NSString *)ruleList;
> 
> Isn't it a little weird to call the argument ruleList since it is a string?

I'll change it.
Comment 6 Sam Weinig 2015-01-18 13:54:08 PST
Committed r178634: <http://trac.webkit.org/changeset/178634>