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

Issue 5088751004942336: Issue 370 - Right-clicked element is removed independent of created filter (Closed)

Created:
Jan. 10, 2014, 10:27 a.m. by Thomas Greiner
Modified:
April 22, 2015, 9:04 a.m.
Visibility:
Public.

Description

See https://adblockplus.org/forum/viewtopic.php?f=11&t=20408 I reused should-collapse to check whether the element should be removed or not. In addition to fixing this issue I also discovered an oversight from the Safari merge (|request| variable was renamed to |msg| but not all references were updated) which I corrected in this review.

Patch Set 1 #

Total comments: 9

Patch Set 2 : Rebase to rev 3c9cea80c481 #

Patch Set 3 : #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -12 lines) Patch
M block.js View 1 2 3 chunks +4 lines, -4 lines 1 comment Download
M include.postload.js View 1 2 4 chunks +39 lines, -7 lines 6 comments Download
M include.preload.js View 1 2 2 chunks +3 lines, -1 line 0 comments Download

Messages

Total messages: 14
Thomas Greiner
Jan. 10, 2014, 10:33 a.m. (2014-01-10 10:33:16 UTC) #1
Sebastian Noack
LGTM
Jan. 10, 2014, 11:02 a.m. (2014-01-10 11:02:23 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/block.js File block.js (right): http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/block.js#newcode83 block.js:83: filters: filters Why send the filters and have the ...
Jan. 10, 2014, 4:28 p.m. (2014-01-10 16:28:56 UTC) #3
Sebastian Noack
http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js#newcode591 include.postload.js:591: }); The problem is not (only) that the element ...
June 25, 2014, 4:40 p.m. (2014-06-25 16:40:58 UTC) #4
Wladimir Palant
http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js#newcode591 include.postload.js:591: }); On 2014/06/25 16:40:59, Sebastian Noack wrote: > What ...
June 25, 2014, 6:49 p.m. (2014-06-25 18:49:34 UTC) #5
Sebastian Noack
http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js#newcode591 include.postload.js:591: }); On 2014/06/25 18:49:34, Wladimir Palant wrote: > On ...
June 25, 2014, 7:05 p.m. (2014-06-25 19:05:05 UTC) #6
Thomas Greiner
http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/block.js File block.js (right): http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/block.js#newcode83 block.js:83: filters: filters On 2014/01/10 16:28:56, Wladimir Palant wrote: > ...
July 17, 2014, 3:35 p.m. (2014-07-17 15:35:49 UTC) #7
Wladimir Palant
http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/5629499534213120/include.postload.js#newcode591 include.postload.js:591: }); On 2014/07/17 15:35:49, Thomas Greiner wrote: > This ...
July 17, 2014, 3:55 p.m. (2014-07-17 15:55:09 UTC) #8
Thomas Greiner
The element hiding filter check is now in. Additionally, it was quite easy to apply ...
July 18, 2014, 9:24 a.m. (2014-07-18 09:24:41 UTC) #9
Wladimir Palant
Sorry about this review taking so long, I actually had it hanging there partially reviewed ...
Nov. 17, 2014, 7:53 p.m. (2014-11-17 19:53:57 UTC) #10
Sebastian Noack
http://codereview.adblockplus.org/5088751004942336/diff/6313034989436928/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/6313034989436928/include.postload.js#newcode587 include.postload.js:587: element.style.setProperty("display", "none", "important"); On 2014/11/17 19:53:57, Wladimir Palant wrote: ...
Nov. 27, 2014, 12:43 p.m. (2014-11-27 12:43:40 UTC) #11
Thomas Greiner
http://codereview.adblockplus.org/5088751004942336/diff/6313034989436928/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/6313034989436928/include.postload.js#newcode574 include.postload.js:574: setElemhideCSSRules(selectors); On 2014/11/17 19:53:57, Wladimir Palant wrote: > The ...
Nov. 27, 2014, 2:46 p.m. (2014-11-27 14:46:07 UTC) #12
Sebastian Noack
http://codereview.adblockplus.org/5088751004942336/diff/6313034989436928/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/5088751004942336/diff/6313034989436928/include.postload.js#newcode587 include.postload.js:587: element.style.setProperty("display", "none", "important"); On 2014/11/27 14:46:07, Thomas Greiner wrote: ...
Nov. 27, 2014, 3:15 p.m. (2014-11-27 15:15:47 UTC) #13
Sebastian Noack
March 4, 2015, 9:56 p.m. (2015-03-04 21:56:24 UTC) #14
Feel free to close or delete this review now. My patch has been merged instead:
http://codereview.adblockplus.org/5838948538515456

Powered by Google App Engine
This is Rietveld