Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5945877571043328: Issue 705 - Generate element hiding instead request blocking filters for non-HTTP URLs (Closed)

Created:
June 24, 2014, 2:54 p.m. by Sebastian Noack
Modified:
Nov. 4, 2014, 8 a.m.
Visibility:
Public.

Description

Issue 705 - Generate element hiding instead request blocking filters for non-HTTP URLs

Patch Set 1 #

Patch Set 2 : Rebased #

Total comments: 2

Patch Set 3 : Rebased #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -4 lines) Patch
M include.postload.js View 1 2 1 chunk +11 lines, -4 lines 3 comments Download

Messages

Total messages: 9
Sebastian Noack
June 24, 2014, 2:54 p.m. (2014-06-24 14:54:42 UTC) #1
Thomas Greiner
Quite a bit has changed in this file since this review was created. Could you ...
Oct. 10, 2014, 12:04 p.m. (2014-10-10 12:04:44 UTC) #2
Sebastian Noack
On 2014/10/10 12:04:44, Thomas Greiner wrote: > Quite a bit has changed in this file ...
Oct. 23, 2014, 11:16 a.m. (2014-10-23 11:16:59 UTC) #3
Thomas Greiner
On 2014/10/23 11:16:59, Sebastian Noack wrote: > On 2014/10/10 12:04:44, Thomas Greiner wrote: > > ...
Oct. 24, 2014, 3:43 p.m. (2014-10-24 15:43:29 UTC) #4
Wladimir Palant
http://codereview.adblockplus.org/5945877571043328/diff/5649050225344512/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5945877571043328/diff/5649050225344512/include.postload.js#newcode388 include.postload.js:388: addSelector(element.localName + "[" + attribute + '="' + value.replace(/"/g, ...
Oct. 24, 2014, 5:56 p.m. (2014-10-24 17:56:55 UTC) #5
Sebastian Noack
http://codereview.adblockplus.org/5945877571043328/diff/5649050225344512/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5945877571043328/diff/5649050225344512/include.postload.js#newcode388 include.postload.js:388: addSelector(element.localName + "[" + attribute + '="' + value.replace(/"/g, ...
Oct. 30, 2014, 4:36 p.m. (2014-10-30 16:36:24 UTC) #6
Wladimir Palant
http://codereview.adblockplus.org/5945877571043328/diff/5657382461898752/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5945877571043328/diff/5657382461898752/include.postload.js#newcode412 include.postload.js:412: var selector = src && elt.localName + '[src=' + ...
Oct. 30, 2014, 10:36 p.m. (2014-10-30 22:36:28 UTC) #7
Sebastian Noack
http://codereview.adblockplus.org/5945877571043328/diff/5657382461898752/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5945877571043328/diff/5657382461898752/include.postload.js#newcode412 include.postload.js:412: var selector = src && elt.localName + '[src=' + ...
Oct. 31, 2014, 1:52 p.m. (2014-10-31 13:52:49 UTC) #8
Wladimir Palant
Nov. 3, 2014, 6:14 p.m. (2014-11-03 18:14:27 UTC) #9
LGTM

http://codereview.adblockplus.org/5945877571043328/diff/5657382461898752/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/5945877571043328/diff/5657382461898752/incl...
include.postload.js:412: var selector = src && elt.localName + '[src=' +
quote(src) + ']';
On 2014/10/31 13:52:49, Sebastian Noack wrote:
> The syntax for valid tag names is very restrictive. It seems that none of the
> valid characters needs escaping in CSS. And (at least on Chrome and Safari)
> calling document.createElement() with an invalid name raises an exception.
Tags
> with invalid names given in the HTML source are parsed as  text nodes.

document.createElement("foo:bar") might still be problematic but not addressing
this here is fine with me.

Powered by Google App Engine
This is Rietveld