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

Issue 6308777468887040: Fixed: Background color not restored when context menu clickhide canceled (Closed)

Created:
Nov. 28, 2013, 11:45 a.m. by Thomas Greiner
Modified:
Dec. 18, 2013, 12:33 p.m.
Visibility:
Public.

Description

When using the "Block Element" context menu entry to select an element on a page that should be blocked, the hover phase (used when clickhide is initiated through the icon popup) is being skipped in which currentElement_backgroundColor is being set. This means that when the "Cancel" button is clicked the original background color could not be restored and the red background color remained.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M include.postload.js View 1 chunk +4 lines, -0 lines 2 comments Download

Messages

Total messages: 4
Thomas Greiner
Nov. 28, 2013, 11:49 a.m. (2013-11-28 11:49:50 UTC) #1
Felix Dahlke
LGTM
Dec. 4, 2013, 3:10 p.m. (2013-12-04 15:10:57 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/6308777468887040/diff/5629499534213120/include.postload.js File include.postload.js (right): http://codereview.adblockplus.org/6308777468887040/diff/5629499534213120/include.postload.js#newcode301 include.postload.js:301: if (typeof currentElement_backgroundColor == "undefined") Given that we never ...
Dec. 18, 2013, 11:57 a.m. (2013-12-18 11:57:04 UTC) #3
Thomas Greiner
Dec. 18, 2013, 12:33 p.m. (2013-12-18 12:33:25 UTC) #4
http://codereview.adblockplus.org/6308777468887040/diff/5629499534213120/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/6308777468887040/diff/5629499534213120/incl...
include.postload.js:301: if (typeof currentElement_backgroundColor ==
"undefined")
On 2013/12/18 11:57:05, Wladimir Palant wrote:
> Given that we never reset currentElement_backgroundColor, won't this fail if
we
> ever selected an element on this page?
> 
> I think that we should simply do this during clickhide-new-filter message
> handling, unconditionally.

Done.
http://codereview.adblockplus.org/5661044793933824/

Powered by Google App Engine
This is Rietveld