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

Issue 29935564: Issue 7452 - Do not cache element hiding filter objects by default

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 2 weeks ago by Manish Jethani
Modified:
3 months, 1 week ago
Reviewers:
hub
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Patch Set 1 #

Total comments: 5

Patch Set 2 : Rename filter* to filterText* #

Total comments: 3

Patch Set 3 : Rebase on GitLab MR #24 #

Total comments: 4

Patch Set 4 : Rebase on next #

Patch Set 5 : Rename filter* to filterText* #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -69 lines) Patch
M lib/elemHide.js View 1 2 3 4 7 chunks +51 lines, -48 lines 0 comments Download
M lib/filterClasses.js View 1 2 3 chunks +17 lines, -2 lines 0 comments Download
M lib/filterListener.js View 1 2 2 chunks +7 lines, -7 lines 0 comments Download
M test/elemHide.js View 1 2 3 4 2 chunks +9 lines, -9 lines 0 comments Download
M test/filterListener.js View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 13
Manish Jethani
8 months, 2 weeks ago (2018-11-03 19:54:05 UTC) #1
Manish Jethani
Patch Set 1 The plan is to make Filter objects (1) stateless and (2) ephemeral ...
8 months, 2 weeks ago (2018-11-03 20:01:25 UTC) #2
hub
https://codereview.adblockplus.org/29935564/diff/29935565/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29935564/diff/29935565/lib/elemHide.js#newcode38 lib/elemHide.js:38: * Lookup table, active flag, by filter by domain. ...
8 months ago (2018-11-22 15:59:34 UTC) #3
Manish Jethani
Patch Set 2: Rename filter* to filterText* I've changed the named of the variables as ...
7 months, 4 weeks ago (2018-11-23 02:03:34 UTC) #4
Manish Jethani
On 2018/11/23 02:03:34, Manish Jethani wrote: > I've changed the named of the variables *names
7 months, 4 weeks ago (2018-11-23 02:04:07 UTC) #5
Manish Jethani
By the way, I won't land this until all the other related patches have also ...
7 months, 4 weeks ago (2018-11-23 02:05:43 UTC) #6
hub
https://codereview.adblockplus.org/29935564/diff/29950555/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29935564/diff/29950555/lib/elemHide.js#newcode244 lib/elemHide.js:244: addToFiltersByDomain(Filter.fromText(unconditionalFilterTextForSelector)); So here, we call Filter.fromText() while before we ...
7 months, 3 weeks ago (2018-11-28 23:21:31 UTC) #7
Manish Jethani
https://codereview.adblockplus.org/29935564/diff/29950555/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29935564/diff/29950555/lib/elemHide.js#newcode244 lib/elemHide.js:244: addToFiltersByDomain(Filter.fromText(unconditionalFilterTextForSelector)); On 2018/11/28 23:21:31, hub wrote: > So here, ...
3 months, 1 week ago (2019-04-07 18:05:09 UTC) #8
Manish Jethani
Patch Set 3: Rebase on GitLab MR #24 I have rebased this on a patch ...
3 months, 1 week ago (2019-04-07 18:14:12 UTC) #9
Manish Jethani
https://codereview.adblockplus.org/29935564/diff/29950555/lib/elemHide.js File lib/elemHide.js (right): https://codereview.adblockplus.org/29935564/diff/29950555/lib/elemHide.js#newcode244 lib/elemHide.js:244: addToFiltersByDomain(Filter.fromText(unconditionalFilterTextForSelector)); On 2019/04/07 18:05:09, Manish Jethani wrote: > On ...
3 months, 1 week ago (2019-04-07 18:55:11 UTC) #10
hub
LGTM https://codereview.adblockplus.org/29935564/diff/30040555/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29935564/diff/30040555/lib/filterClasses.js#newcode141 lib/filterClasses.js:141: return !(filter instanceof ElemHideFilter); On 2019/04/07 18:14:11, Manish ...
3 months, 1 week ago (2019-04-08 20:44:54 UTC) #11
Manish Jethani
Patch Set 4: Rebase on next Patch Set 5: Rename filter* to filterText*
3 months, 1 week ago (2019-04-08 22:01:38 UTC) #12
Manish Jethani
3 months, 1 week ago (2019-04-09 02:55:31 UTC) #13
On 2019/04/08 22:01:38, Manish Jethani wrote:
> Patch Set 4: Rebase on next
> Patch Set 5: Rename filter* to filterText*

I have filed a new issue for this since the original ticket is now closed:

https://issues.adblockplus.org/ticket/7452

We still cannot land this because if we do then it would make element hiding
filters unable to retain their state, like whether a filter is disabled, how
many hits it got, and so on. While we do not use these properties in the current
WebExt version, we still support them so they must keep working. I have
described this in the issue.
Sign in to reply to this message.

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