-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not use eval in minified piwik.js, replace JSON2 with JSON3 #8896
Conversation
@@ -5624,7 +6188,9 @@ if (typeof piwik_log !== 'function') { | |||
|
|||
function getOption(optionName) { | |||
try { | |||
return eval('piwik_' + optionName); | |||
if (window['piwik_' + optionName]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how the old tracker used to work but this should be the same
After merging it, I'd run the tests on some older browsers via saucelabs on top to make sure it works with IE6 etc |
@@ -439,10 +439,14 @@ function PiwikTest() { | |||
var src = '<?php | |||
$src = file_get_contents('../../js/piwik.js'); | |||
$src = strtr($src, array('\\'=>'\\\\',"'"=>"\\'",'"'=>'\\"',"\r"=>'\\r',"\n"=>'\\n','</'=>'<\/')); | |||
$src = substr($src, strpos($src, '/* startjslint */')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a workaround to not run JSON3 through JSLINT as we cannot make it JSLINT compatible
Do not use eval in minified piwik.js, replace JSON2 with JSON3
Nice. Next step: #1542 |
FYI: Ran JSON tests on IE6 and IE7 and worked. Not all other tests don't work because they are not compatible with older browsers |
fixes #5960 refs #1542