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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 8 months ago by Thomas Greiner
Modified:
4 years, 5 months ago
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
5 years, 8 months ago (2014-01-10 10:33:16 UTC) #1
Sebastian Noack
LGTM
5 years, 8 months ago (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 ...
5 years, 8 months ago (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 ...
5 years, 2 months ago (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 ...
5 years, 2 months ago (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 ...
5 years, 2 months ago (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: > ...
5 years, 2 months ago (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 ...
5 years, 2 months ago (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 ...
5 years, 2 months ago (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 ...
4 years, 10 months ago (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: ...
4 years, 9 months ago (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 ...
4 years, 9 months ago (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: ...
4 years, 9 months ago (2014-11-27 15:15:47 UTC) #13
Sebastian Noack
4 years, 6 months ago (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
Sign in to reply to this message.

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