adblockpluschrome: Handle invalid share popup sites

[Wladimir Palant, 2012-10-25 15:32:51]

http://codereview.adblockplus.org/8689042/diff/1/firstRun.js
File firstRun.js (right):

http://codereview.adblockplus.org/8689042/diff/1/firstRun.js#newcode50
firstRun.js:50: }, 2000);
This timeout should be 20 seconds at least, otherwise we exclude people in China
and such. But on a second thought - do we even need that timeout? There will
always be a "load" event, even if it is an error page. If we don't receive the
message then we just close the pop-up (call popupCloseListener without
registering it) and we are done with it.

[Felix Dahlke, 2012-10-25 15:39:27]

New patch set uploaded.

http://codereview.adblockplus.org/8689042/diff/1/firstRun.js
File firstRun.js (right):

http://codereview.adblockplus.org/8689042/diff/1/firstRun.js#newcode50
firstRun.js:50: }, 2000);
On 2012/10/25 15:32:51, Wladimir Palant wrote:
> This timeout should be 20 seconds at least, otherwise we exclude people in
China
> and such. But on a second thought - do we even need that timeout? There will
> always be a "load" event, even if it is an error page. If we don't receive the
> message then we just close the pop-up (call popupCloseListener without
> registering it) and we are done with it.

You're right, the load event will even arrive eventually when the user is
offline.

[Wladimir Palant, 2012-10-25 15:54:15]

LGTM with the one nit fixed

http://codereview.adblockplus.org/8689042/diff/5001/firstRun.js
File firstRun.js (right):

http://codereview.adblockplus.org/8689042/diff/5001/firstRun.js#newcode38
firstRun.js:38: glassPane.className = "";
Also remove message listener here?

[Felix Dahlke, 2012-10-25 15:57:11]

Okay.

http://codereview.adblockplus.org/8689042/diff/5001/firstRun.js
File firstRun.js (right):

http://codereview.adblockplus.org/8689042/diff/5001/firstRun.js#newcode38
firstRun.js:38: glassPane.className = "";
On 2012/10/25 15:54:15, Wladimir Palant wrote:
> Also remove message listener here?

Done.