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

Unified Diff: lib/elemHide.js

Issue 29342837: Issue 4054 - Remove filters from filtersByDomain (Closed)
Patch Set: Created May 20, 2016, 11:12 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/elemHide.js
diff --git a/lib/elemHide.js b/lib/elemHide.js
index 6e2f1b56acbb6e67e79f779503b469f9d8829903..3d5bf2f5ea33972a7065d6622596443d999602da 100644
--- a/lib/elemHide.js
+++ b/lib/elemHide.js
@@ -202,6 +202,17 @@ var ElemHide = exports.ElemHide =
delete keyByFilter[filter.text];
ElemHide.isDirty = true;
}
+
+ if (usingFiltersByDomain)
+ {
+ let domains = Object.keys(filter.domains || defaultDomains);
Sebastian Noack 2016/05/20 12:20:44 So in the case here for..in would be actually slow
kzar 2016/05/20 12:49:29 Well I didn't think about it either way to be hone
kzar 2016/05/20 13:13:38 As we discussed in IRC I did a quick test of this
+ for (let domain of domains)
+ {
+ let filters = filtersByDomain[domain];
+ if (filters)
+ delete filters[filter.text];
kzar 2016/05/20 11:36:12 Note: There is a memory leak here. I don't delete
Wladimir Palant 2016/05/20 12:17:51 I don't think that this is a big deal. All filters
Sebastian Noack 2016/05/20 12:20:44 How did you try to delete filtersByDomain[domain]?
Sebastian Noack 2016/05/20 12:29:24 What if an element hiding filter is given in two s
Wladimir Palant 2016/05/20 12:40:05 Frankly, I don't care. No matter how you do it, it
Wladimir Palant 2016/05/20 12:40:05 That case is being handled elsewhere (FilterListen
kzar 2016/05/20 12:49:29 Whoops sorry, I moved this at the last minute and
kzar 2016/05/20 12:49:29 I didn't consider that. I'm not sure how we can ha
kzar 2016/05/20 12:49:29 Well I tried those ideas already except using .get
+ }
+ }
},
/**
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld