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

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

Created:
Nov. 3, 2018, 7:54 p.m. by Manish Jethani
Modified:
April 9, 2019, 2:55 a.m.
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
Nov. 3, 2018, 7:54 p.m. (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 ...
Nov. 3, 2018, 8:01 p.m. (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. ...
Nov. 22, 2018, 3:59 p.m. (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 ...
Nov. 23, 2018, 2:03 a.m. (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
Nov. 23, 2018, 2:04 a.m. (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 ...
Nov. 23, 2018, 2:05 a.m. (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 ...
Nov. 28, 2018, 11:21 p.m. (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, ...
April 7, 2019, 6:05 p.m. (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 ...
April 7, 2019, 6:14 p.m. (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 ...
April 7, 2019, 6:55 p.m. (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 ...
April 8, 2019, 8:44 p.m. (2019-04-08 20:44:54 UTC) #11
Manish Jethani
Patch Set 4: Rebase on next Patch Set 5: Rename filter* to filterText*
April 8, 2019, 10:01 p.m. (2019-04-08 22:01:38 UTC) #12
Manish Jethani
April 9, 2019, 2:55 a.m. (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.

Powered by Google App Engine
This is Rietveld