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

Issue 6680224334872576: Issue 303 - Don't load element hiding rules (Closed)

Created:
Nov. 6, 2014, 1:36 p.m. by René Jeschke
Modified:
Feb. 4, 2015, 1:07 p.m.
Reviewers:
Felix Dahlke
CC:
Wladimir Palant
Visibility:
Public.

Description

Issue 303 - Don't load element hiding rules

Patch Set 1 #

Total comments: 7

Patch Set 2 : getPath(), char-by-char #

Patch Set 3 : Removed mightBeFilterList boolean var. #

Patch Set 4 : Added check for valid, listed subscription. #

Patch Set 5 : Made HACK obvious. #

Total comments: 18

Patch Set 6 : Naming, FIXME, whitelist-url #

Total comments: 1

Patch Set 7 : URL query removal #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -25 lines) Patch
M src/org/adblockplus/android/ABPEngine.java View 1 2 3 4 5 3 chunks +4 lines, -2 lines 0 comments Download
M src/org/adblockplus/android/AndroidWebRequest.java View 1 2 3 4 5 6 3 chunks +58 lines, -23 lines 0 comments Download

Messages

Total messages: 19
René Jeschke
Nov. 6, 2014, 1:41 p.m. (2014-11-06 13:41:09 UTC) #1
Felix Dahlke
http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java#newcode53 src/org/adblockplus/android/AndroidWebRequest.java:53: filename = filename.substring(0, params); Why not just use url.getPath() ...
Nov. 11, 2014, 9:15 a.m. (2014-11-11 09:15:36 UTC) #2
René Jeschke
http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java#newcode53 src/org/adblockplus/android/AndroidWebRequest.java:53: filename = filename.substring(0, params); On 2014/11/11 09:15:37, Felix H. ...
Nov. 11, 2014, 11:55 a.m. (2014-11-11 11:55:55 UTC) #3
Felix Dahlke
http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java#newcode56 src/org/adblockplus/android/AndroidWebRequest.java:56: return filename.toLowerCase(Locale.US).trim().endsWith(".txt"); On 2014/11/11 11:55:55, René Jeschke wrote: > ...
Nov. 11, 2014, 4:18 p.m. (2014-11-11 16:18:30 UTC) #4
René Jeschke
On 2014/11/11 16:18:30, Felix H. Dahlke wrote: > http://codereview.adblockplus.org/6680224334872576/diff/5629499534213120/src/org/adblockplus/android/AndroidWebRequest.java > File src/org/adblockplus/android/AndroidWebRequest.java (right): > > ...
Nov. 11, 2014, 4:34 p.m. (2014-11-11 16:34:53 UTC) #5
René Jeschke
Nov. 11, 2014, 4:35 p.m. (2014-11-11 16:35:04 UTC) #6
Felix Dahlke
On 2014/11/11 16:34:53, René Jeschke wrote: > On 2014/11/11 16:18:30, Felix H. Dahlke wrote: > ...
Nov. 13, 2014, 9:44 a.m. (2014-11-13 09:44:21 UTC) #7
René Jeschke
On 2014/11/13 09:44:21, Felix H. Dahlke wrote: > On 2014/11/11 16:34:53, René Jeschke wrote: > ...
Nov. 17, 2014, 10:15 a.m. (2014-11-17 10:15:14 UTC) #8
Felix Dahlke
On 2014/11/17 10:15:14, René Jeschke wrote: > On 2014/11/13 09:44:21, Felix H. Dahlke wrote: > ...
Nov. 17, 2014, 1:34 p.m. (2014-11-17 13:34:08 UTC) #9
René Jeschke
On 2014/11/17 13:34:08, Felix H. Dahlke wrote: > On 2014/11/17 10:15:14, René Jeschke wrote: > ...
Nov. 17, 2014, 1:36 p.m. (2014-11-17 13:36:48 UTC) #10
René Jeschke
On 2014/11/17 13:36:48, René Jeschke wrote: > On 2014/11/17 13:34:08, Felix H. Dahlke wrote: > ...
Nov. 18, 2014, 6:11 p.m. (2014-11-18 18:11:40 UTC) #11
Felix Dahlke
Yeah, I prefer this to my suggestion actually, was a bit overgeneralised... Some comments though. ...
Nov. 19, 2014, 10:16 a.m. (2014-11-19 10:16:37 UTC) #12
René Jeschke
http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java#newcode40 src/org/adblockplus/android/AndroidWebRequest.java:40: private final HashSet<String> subscriptionURLs = new HashSet<String>(); On 2014/11/19 ...
Nov. 24, 2014, 1:15 p.m. (2014-11-24 13:15:57 UTC) #13
Felix Dahlke
http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java#newcode40 src/org/adblockplus/android/AndroidWebRequest.java:40: private final HashSet<String> subscriptionURLs = new HashSet<String>(); On 2014/11/24 ...
Nov. 27, 2014, 5:41 a.m. (2014-11-27 05:41:56 UTC) #14
René Jeschke
http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java#newcode62 src/org/adblockplus/android/AndroidWebRequest.java:62: for (final org.adblockplus.libadblockplus.Subscription s : engine.fetchAvailableSubscriptions()) On 2014/11/27 05:41:57, ...
Jan. 23, 2015, 1:28 p.m. (2015-01-23 13:28:10 UTC) #15
Felix Dahlke
http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java File src/org/adblockplus/android/AndroidWebRequest.java (right): http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java#newcode95 src/org/adblockplus/android/AndroidWebRequest.java:95: // FIXME This is a hack! On 2015/01/23 13:28:10, ...
Jan. 27, 2015, 8:40 a.m. (2015-01-27 08:40:51 UTC) #16
René Jeschke
On 2015/01/27 08:40:51, Felix H. Dahlke wrote: > http://codereview.adblockplus.org/6680224334872576/diff/5126842331693056/src/org/adblockplus/android/AndroidWebRequest.java > File src/org/adblockplus/android/AndroidWebRequest.java (right): > > ...
Feb. 4, 2015, 12:05 p.m. (2015-02-04 12:05:25 UTC) #17
René Jeschke
Feb. 4, 2015, 12:09 p.m. (2015-02-04 12:09:05 UTC) #18
Felix Dahlke
Feb. 4, 2015, 1:06 p.m. (2015-02-04 13:06:14 UTC) #19
LGTM

I'm currently assuming we can just get rid of this hack once we've fixed the
element hiding issues on Android. With Typed Objects, the memory overhead caused
by element hiding should hopefully be non-significant.

Powered by Google App Engine
This is Rietveld