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

Issue 29336278: Issue 3651 - Fix popup blocking with dynamically created openers and invisible redirects (Closed)

Created:
Feb. 11, 2016, 5:22 p.m. by Sebastian Noack
Modified:
Feb. 12, 2016, 4:10 p.m.
Reviewers:
kzar, Wladimir Palant
Visibility:
Public.

Description

Issue 3651 - Fix popup blocking with dynamically created openers and invisible redirects

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -39 lines) Patch
M lib/popupBlocker.js View 2 chunks +118 lines, -39 lines 4 comments Download

Messages

Total messages: 5
Sebastian Noack
Feb. 11, 2016, 5:24 p.m. (2016-02-11 17:24:39 UTC) #1
kzar
I feel like I should say more as it's a big change, but after looking ...
Feb. 12, 2016, 12:46 p.m. (2016-02-12 12:46:31 UTC) #2
Wladimir Palant
After understanding the implicit assumptions, this looks good. Still, two questions below. https://codereview.adblockplus.org/29336278/diff/29336279/lib/popupBlocker.js File lib/popupBlocker.js ...
Feb. 12, 2016, 3:43 p.m. (2016-02-12 15:43:44 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29336278/diff/29336279/lib/popupBlocker.js File lib/popupBlocker.js (right): https://codereview.adblockplus.org/29336278/diff/29336279/lib/popupBlocker.js#newcode32 lib/popupBlocker.js:32: return Object.keys(loadingPopups).length > 0; On 2016/02/12 15:43:43, Wladimir Palant ...
Feb. 12, 2016, 3:52 p.m. (2016-02-12 15:52:13 UTC) #4
Wladimir Palant
Feb. 12, 2016, 3:53 p.m. (2016-02-12 15:53:16 UTC) #5
Ok, LGTM

Powered by Google App Engine
This is Rietveld