Issue 1610 - Don't remove overlays before the "Block element" dialog is closed

Issue 1610 - Don't remove overlays before the "Block element" dialog is closed

[Sebastian Noack, 2014-11-26 12:03:29]

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
File include.postload.js (left):

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:315: // For some reason iterating over the array returend by
getElementsByClassName() doesn't work
It's not an Array, but a NodeList, and therefore lazy evaluated. Hence when you
remove the first element, the second element becomes the first element. But you
can still use getElementsByClassName() and iterate over the first element of the
NodeList until its empty. While this is unrelated to what this patch is actually
about, I felt free to address this below.

[kzar, 2014-11-26 14:14:04]

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:284: function clickHide_deactivate(keepOverlays)
We're relying on the legacy code not passing this parameter and it therefore
being undefined?

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:303: if(currentElement) {
Nitpick: Should be space between "if" and "("?

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:304: currentElement.removeEventListener("contextmenu",
clickHide_elementClickHandler, false);
Last parameter should be true?

[Sebastian Noack, 2014-11-26 14:41:38]

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:284: function clickHide_deactivate(keepOverlays)
On 2014/11/26 14:14:04, kzar wrote:
> We're relying on the legacy code not passing this parameter and it therefore
> being undefined?

Correct. I tried to keep the patch as compact as possible this time.

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:303: if(currentElement) {
On 2014/11/26 14:14:04, kzar wrote:
> Nitpick: Should be space between "if" and "("?

I just copied the code from above, but I have added the space now.

http://codereview.adblockplus.org/4923599135703040/diff/5629499534213120/incl...
include.postload.js:304: currentElement.removeEventListener("contextmenu",
clickHide_elementClickHandler, false);
On 2014/11/26 14:14:04, kzar wrote:
> Last parameter should be true?

Yes, this patch was created on top of a version, were it still be false. Should
be fixed after rebase.

[kzar, 2014-11-26 14:55:12]

LGTM