Addressed comment on review 6308777468887040

include.postload.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 298 matching lines @@
   }
   if (url)
   {
     clickHideFilters.push(relativeToAbsoluteUrl(url));
     selectorList.push(elt.localName + '[src="' + url + '"]');
   }
 
   // Show popup
   clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
 
-  // Set background color in case it got selected using context menu
-  if (typeof currentElement_backgroundColor == "undefined")
-    currentElement_backgroundColor = currentElement.style.backgroundColor;
-
   // Highlight the unlucky elements
   // Restore currentElement's box-shadow and bgcolor so that highlightElements won't save those
   currentElement.style.setProperty("-webkit-box-shadow", currentElement_boxShadow);
   currentElement.style.backgroundColor = currentElement_backgroundColor;
   // Highlight the elements specified by selector in yellow
   highlightElements(selectorList.join(","));
   // Now, actually highlight the element the user clicked on in red
   currentElement.style.setProperty("-webkit-box-shadow", "inset 0px 0px 5px #fd1708");
   currentElement.style.backgroundColor = "#f6a1b5";
 
@@ skipping 214 matching lines @@
         if(msg.filter === url)
         {
           // This request would have come from the chrome.contextMenu handler, so we
           // simulate the user having chosen the element to get rid of via the usual means.
           clickHide_activated = true;
           // FIXME: clickHideFilters is erased in clickHide_mouseClick anyway, so why set it?
           clickHideFilters = [msg.filter];
           // Coerce red highlighted overlay on top of element to remove.
           // TODO: Wow, the design of the clickHide stuff is really dumb - gotta fix it sometime
           currentElement = addElementOverlay(target);
+          currentElement_backgroundColor = currentElement.style.backgroundColor;

[Wladimir Palant, 2013/12/18 12:19:25]

From the look of it, this won't work correctly. You are remembering the
background color of the temporary overlay element. clickHide_showDialog() will
remove that overlay and restore the wrong background color later. IMHO this
should be target.style.backgroundColor.

[Thomas Greiner, 2013/12/18 12:24:02]

Done, you are right. We should change the variable name at some point given that
the overlay is called currentElement but currentElement_backgroundColor is not
the background color of the overlay.

           // clickHide_mouseOver(lastRightClickEvent);
           clickHide_mouseClick(lastRightClickEvent);
         }
         else
           console.log("clickhide-new-filter: URLs don't match. Couldn't find that element.", request.filter, url, lastRightClickEvent.target.src);
         break;
       case "clickhide-init":
         if (clickHideFiltersDialog)
         {
           sendResponse({filters: clickHide_filters});
@@ skipping 19 matching lines @@
 
           clickHide_deactivate();
         }
         break;
       default:
         sendResponse({});
         break;
     }
   });
 }