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

Issue 8684120: Fixed: Topic 11234 - toolbar icon not shown in popup windows (Closed)

Created:
Oct. 30, 2012, 5:12 p.m. by Thomas Greiner
Modified:
Oct. 31, 2012, 9:28 a.m.
Reviewers:
Wladimir Palant
Visibility:
Public.

Description

Fixed: Topic 11234 - toolbar icon not shown in popup windows

Patch Set 1 #

Total comments: 4

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -3 lines) Patch
M lib/windowObserver.js View 1 3 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 5
Thomas Greiner
Oct. 30, 2012, 5:12 p.m. (2012-10-30 17:12:09 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/8684120/diff/1/lib/windowObserver.js File lib/windowObserver.js (right): http://codereview.adblockplus.org/8684120/diff/1/lib/windowObserver.js#newcode37 lib/windowObserver.js:37: Services.obs.addObserver(this, "chrome-document-global-created", false); Please use weak references just in ...
Oct. 30, 2012, 5:20 p.m. (2012-10-30 17:20:20 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/8684120/diff/1/lib/windowObserver.js File lib/windowObserver.js (right): http://codereview.adblockplus.org/8684120/diff/1/lib/windowObserver.js#newcode68 lib/windowObserver.js:68: if (subject.location.href !== "about:blank") Forgot to mention: there should ...
Oct. 30, 2012, 6:52 p.m. (2012-10-30 18:52:21 UTC) #3
Thomas Greiner
Oct. 31, 2012, 9:07 a.m. (2012-10-31 09:07:15 UTC) #4
Wladimir Palant
Oct. 31, 2012, 9:19 a.m. (2012-10-31 09:19:20 UTC) #5
LGTM

Powered by Google App Engine
This is Rietveld