Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1183)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 8 months ago by René Jeschke
Modified:
4 years, 5 months ago
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
4 years, 8 months ago (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() ...
4 years, 8 months ago (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. ...
4 years, 8 months ago (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: > ...
4 years, 8 months ago (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): > > ...
4 years, 8 months ago (2014-11-11 16:34:53 UTC) #5
René Jeschke
4 years, 8 months ago (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: > ...
4 years, 8 months ago (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: > ...
4 years, 8 months ago (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: > ...
4 years, 8 months ago (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: > ...
4 years, 8 months ago (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: > ...
4 years, 8 months ago (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. ...
4 years, 8 months ago (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 ...
4 years, 7 months ago (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 ...
4 years, 7 months ago (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, ...
4 years, 5 months ago (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, ...
4 years, 5 months ago (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): > > ...
4 years, 5 months ago (2015-02-04 12:05:25 UTC) #17
René Jeschke
4 years, 5 months ago (2015-02-04 12:09:05 UTC) #18
Felix Dahlke
4 years, 5 months ago (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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5