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

Issue 29445594: Noissue - Fix some problems Firefox has with our injected code (Closed)

Created:
May 22, 2017, 10:31 a.m. by kzar
Modified:
May 22, 2017, 1:09 p.m.
Visibility:
Public.

Description

Noissue - Fix some problems Firefox has with our injected code

Patch Set 1 #

Patch Set 2 : Removed extra newline which was added by mistake #

Total comments: 2

Patch Set 3 : Don't copy the caller property #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -2 lines) Patch
M inject.preload.js View 1 2 2 chunks +7 lines, -2 lines 2 comments Download

Messages

Total messages: 8
kzar
Patch Set 1
May 22, 2017, 10:33 a.m. (2017-05-22 10:33:30 UTC) #1
kzar
Patch Set 2 : Removed extra newline which was added by mistake
May 22, 2017, 10:36 a.m. (2017-05-22 10:36:35 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29445594/diff/29445597/inject.preload.js File inject.preload.js (right): https://codereview.adblockplus.org/29445594/diff/29445597/inject.preload.js#newcode380 inject.preload.js:380: ["caller", "generateCertificate", "name", "prototype"]); So the issue is being ...
May 22, 2017, 10:55 a.m. (2017-05-22 10:55:17 UTC) #3
kzar
Patch Set 3 : Don't copy the caller property I've not been able to properly ...
May 22, 2017, 11:25 a.m. (2017-05-22 11:25:31 UTC) #4
Wladimir Palant
LGTM
May 22, 2017, 12:03 p.m. (2017-05-22 12:03:23 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29445594/diff/29445599/inject.preload.js File inject.preload.js (left): https://codereview.adblockplus.org/29445594/diff/29445599/inject.preload.js#oldcode361 inject.preload.js:361: let configuration = protectConfiguration(args[0]); Nit: Perhaps move this definition ...
May 22, 2017, 12:16 p.m. (2017-05-22 12:16:55 UTC) #6
kzar
https://codereview.adblockplus.org/29445594/diff/29445599/inject.preload.js File inject.preload.js (left): https://codereview.adblockplus.org/29445594/diff/29445599/inject.preload.js#oldcode361 inject.preload.js:361: let configuration = protectConfiguration(args[0]); On 2017/05/22 12:16:54, Sebastian Noack ...
May 22, 2017, 1:02 p.m. (2017-05-22 13:02:04 UTC) #7
Sebastian Noack
May 22, 2017, 1:03 p.m. (2017-05-22 13:03:22 UTC) #8
Fair enough, LGTM.

Powered by Google App Engine
This is Rietveld