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

Issue 29336233: Issue 3568 - Block pop-ups opened via an intermediate window (Closed)

Created:
Feb. 10, 2016, 3:10 p.m. by Wladimir Palant
Modified:
Feb. 11, 2016, 1:46 p.m.
Reviewers:
Thomas Greiner
CC:
Sebastian Noack
Visibility:
Public.

Description

This is ugly - rather than relying on the browser to give us the opener we prefer to remember it ourselves, particularly for redirects that wouldn`t have this info otherwise. The consequence is that any redirect might trigger the pop-up blocker, no matter how long ago the pop-up was opened (as long as the opener window is still around). I guess that`s ok...

Patch Set 1 #

Patch Set 2 : Fixed small logic flaw #

Total comments: 2

Patch Set 3 : Fixed nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -5 lines) Patch
M lib/child/contentPolicy.js View 1 2 2 chunks +22 lines, -5 lines 0 comments Download

Messages

Total messages: 3
Wladimir Palant
Feb. 10, 2016, 3:10 p.m. (2016-02-10 15:10:08 UTC) #1
Thomas Greiner
LGTM https://codereview.adblockplus.org/29336233/diff/29336239/lib/child/contentPolicy.js File lib/child/contentPolicy.js (right): https://codereview.adblockplus.org/29336233/diff/29336239/lib/child/contentPolicy.js#newcode319 lib/child/contentPolicy.js:319: var opener = this._openers.get(subject); Detail: Usually, we use ...
Feb. 11, 2016, 11:06 a.m. (2016-02-11 11:06:23 UTC) #2
Wladimir Palant
Feb. 11, 2016, 1:44 p.m. (2016-02-11 13:44:45 UTC) #3
https://codereview.adblockplus.org/29336233/diff/29336239/lib/child/contentPo...
File lib/child/contentPolicy.js (right):

https://codereview.adblockplus.org/29336233/diff/29336239/lib/child/contentPo...
lib/child/contentPolicy.js:319: var opener = this._openers.get(subject);
On 2016/02/11 11:06:23, Thomas Greiner wrote:
> Detail: Usually, we use `let` over `var` to define variables.

Done.

Powered by Google App Engine
This is Rietveld