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

Issue 8741005: Topic 11337 - More reliable approach to hide image/frame placeholders (Closed)

Created:
Oct. 31, 2012, 9:14 a.m. by Wladimir Palant
Modified:
Nov. 1, 2012, 7:54 a.m.
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

I meant to use the error event from the start but got confused because that event doesn`t bubble - I forgot that a capturing event listener can get it at the top level nevertheless. This works really nicely for images, for frames we have to look at the load event however and check even frames that loaded properly (no way to distinguish them from a content script).

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -3 lines) Patch
M .hgsubstate View 1 1 chunk +1 line, -1 line 0 comments Download
M background.js View 1 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 4
Wladimir Palant
Oct. 31, 2012, 9:14 a.m. (2012-10-31 09:14:35 UTC) #1
Thomas Greiner
LGTM only one variable name that is confusing http://codereview.adblockplus.org/8741005/diff/1/background.js File background.js (right): http://codereview.adblockplus.org/8741005/diff/1/background.js#newcode541 background.js:541: if ...
Oct. 31, 2012, 11:32 a.m. (2012-10-31 11:32:00 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/8741005/diff/1/background.js File background.js (right): http://codereview.adblockplus.org/8741005/diff/1/background.js#newcode541 background.js:541: if (!enabled) That's code copied from element hiding handling ...
Oct. 31, 2012, 12:17 p.m. (2012-10-31 12:17:53 UTC) #3
Wladimir Palant
Oct. 31, 2012, 12:20 p.m. (2012-10-31 12:20:31 UTC) #4

          

Powered by Google App Engine
This is Rietveld