This patch adds support for Chrome's match patterns [1] rather than regexps for URL filtering, which is what we're using now. I also fixed a random bug. [1] https://developer.chrome.com/extensions/match_patterns

Comment on attachment 8614968 [details] [diff] [review] patch Review of attachment 8614968 [details] [diff] [review]: ----------------------------------------------------------------- This really needs some MatchPattern tests before it should land. ::: toolkit/modules/addons/MatchPattern.jsm @@ +9,5 @@ > +const PERMITTED_SCHEMES = ["http", "https", "file", "ftp"]; > + > +// This function converts a glob pattern (containing * and possibly ? > +// as wildcards) to a regular expression. > +function globToRegexp(pat, allowQuestion) allowQuestion is never true. Do we need it? @@ +20,5 @@ > + } else { > + pat = pat.replace(/\?/g, "\\?"); > + } > + pat = pat.replace(/\*/g, ".*"); > + return new RegExp(pat); new RegExp("^" + pat + "$") @@ +34,5 @@ > + this.path = new RegExp('.*'); > + } else if (!pat) { > + this.scheme = []; > + } else { > + let re = new RegExp("^(http|https|file|ftp|\\*):\\/\\/(\\*|\\*\\.[^*/]+|[^*/]+)/(.*)$"); You don't need to escape the / characters in here @@ +42,5 @@ > + this.scheme = []; > + } > + > + if (match[1] == '*') { > + this.scheme = PERMITTED_SCHEMES; This doesn't match Chrome's behaviour which only allows https or http for '*' @@ +46,5 @@ > + this.scheme = PERMITTED_SCHEMES; > + } else { > + this.scheme = [match[1]]; > + } > + this.host = globToRegexp(match[2], false); The alternative to generating a full regexp for the host is that it's always either an exact string match or a suffix match. Maybe not worth changing since you had to write this code for the path case anyway. @@ +57,5 @@ > + if (this.scheme.indexOf(uri.scheme) == -1) { > + return false; > + } > + > + if (!this.host.exec(uri.host)) { The Chrome docs aren't specific about what to do in the case of ports being present. Maybe hostPort is a better choice here? @@ +61,5 @@ > + if (!this.host.exec(uri.host)) { > + return false; > + } > + > + if (!this.path.exec(uri.path)) { Here and about, test is quicker. @@ +81,5 @@ > + } > +} > + > +MatchPattern.prototype = { > + matches(uri) { Better add a note that this expects an nsIURI @@ +86,5 @@ > + for (let matcher of this.matchers) { > + if (matcher.matches(uri)) { > + return true; > + } > + return false; This is only checking the first matcher!

Thanks Dave! The allowQuestion bit is to support glob patterns: https://developer.chrome.com/extensions/content_scripts#match-patterns-globs I haven't actually implemented this yet, but I'd like to do so. It seems like the code is worth keeping.

I've updated the examples in WebRequest.jsm to use MatchPatterns, and added a little section on how to construct match patterns, and pointed to the match patterns doc for details of the string syntax. I'll mark this dev-doc-complete, but let me know if you see anything else that needs adding.

Thanks!