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

Issue 29329779: Issue 3258 - Blockable items: restore item flashing functionality (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 7 months ago by Wladimir Palant
Modified:
4 years, 6 months ago
Visibility:
Public.

Description

Issue 3258 - Blockable items: restore item flashing functionality

Patch Set 1 #

Patch Set 2 : Better handling of dead objects #

Total comments: 4

Patch Set 3 : Addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -54 lines) Patch
M chrome/content/ui/sidebar.js View 4 chunks +4 lines, -5 lines 0 comments Download
M chrome/content/ui/sidebar.xul View 1 chunk +0 lines, -1 line 0 comments Download
M lib/child/flasher.js View 1 2 1 chunk +36 lines, -45 lines 0 comments Download
M lib/child/requestNotifier.js View 1 2 6 chunks +62 lines, -3 lines 0 comments Download
M lib/requestNotifier.js View 1 chunk +18 lines, -0 lines 0 comments Download

Messages

Total messages: 4
Wladimir Palant
4 years, 7 months ago (2015-11-05 20:52:39 UTC) #1
Thomas Greiner
LGTM, just minor remarks on documentation https://codereview.adblockplus.org/29329779/diff/29329791/lib/child/flasher.js File lib/child/flasher.js (right): https://codereview.adblockplus.org/29329779/diff/29329791/lib/child/flasher.js#newcode27 lib/child/flasher.js:27: nodes[0].scrollIntoView(); Detail: You're ...
4 years, 6 months ago (2015-11-24 12:15:43 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29329779/diff/29329791/lib/child/flasher.js File lib/child/flasher.js (right): https://codereview.adblockplus.org/29329779/diff/29329791/lib/child/flasher.js#newcode27 lib/child/flasher.js:27: nodes[0].scrollIntoView(); On 2015/11/24 12:15:42, Thomas Greiner wrote: > Detail: ...
4 years, 6 months ago (2015-11-25 19:06:39 UTC) #3
Thomas Greiner
4 years, 6 months ago (2015-11-26 10:49:12 UTC) #4
LGTM
Sign in to reply to this message.

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