Skip to content
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 polute global JS namespace #182

Closed
1 task done
kraftner opened this issue Jul 7, 2023 · 4 comments
Closed
1 task done

Do not polute global JS namespace #182

kraftner opened this issue Jul 7, 2023 · 4 comments

Comments

@kraftner
Copy link
Contributor

kraftner commented Jul 7, 2023

Bug/Problem

There currently are multiple JS functions defined in the global namespace. To make things worse they also have very generic names. They are also taken from stackoverflow which makes it even more likely to have collisions with other peoples code.

  • get_cookie
  • htmlentities_decode
  • remove_cookie
  • set_cookie

/**
* Get a cookie.
*
* @link https://stackoverflow.com/a/24103596/3461955
*
* @param {string} name The name of the cookie
*/
function get_cookie( name ) {
var nameEQ = name + '=';
var ca = document.cookie.split( ';' );
for ( var i = 0; i < ca.length; i++ ) {
var c = ca[ i ];
while ( c.charAt( 0 ) == ' ' ) c = c.substring( 1, c.length );
if ( c.indexOf( nameEQ ) == 0 ) return c.substring( nameEQ.length, c.length );
}
return null;
}
/**
* Decode a string with HTML entities.
*
* @param {string} content The content to decode
* @return {string} The decoded content
*/
function htmlentities_decode( content ) {
var textarea = document.createElement( 'textarea' );
textarea.innerHTML = content;
return textarea.value;
}
/**
* Remove a cookie.
*
* @link https://stackoverflow.com/a/24103596/3461955
*
* @param {string} name The name of the cookie
*/
function remove_cookie( name ) {
document.cookie = name + '=; expires=0; path=/';
}
/**
* Set a cookie.
*
* @link https://stackoverflow.com/a/24103596/3461955
*
* @param {string} name The name of the cookie
* @param {string} value The value of the cookie
* @param {number} days The expiration in days
*/
function set_cookie( name, value, days ) {
var expires = '';
if ( days ) {
var date = new Date();
date.setTime( date.getTime() + ( days * 24 * 60 * 60 * 1000 ) );
expires = '; expires=' + date.toUTCString();
}
document.cookie = name + '=' + ( value || '' ) + expires + '; path=/';
}

Steps to reproduce

  1. Go to the frontend of a site with Embed Privacy enabled
  2. Open developer console
  3. Start typing e.g. get_ cookie to see that they are defined in the global namespace.

Version

1.7.2

Link

No response

Environment info

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@kraftner
Copy link
Contributor Author

kraftner commented Jul 7, 2023

The easiest solution probably is to wrap the whole file into an Immediately Invoked Function Expression to have everything in its own scope.
I do not expect any breaking changes to be caused by that. If you want I can also offer to create a PR for that.

@MatzeKitt
Copy link
Member

MatzeKitt commented Jul 7, 2023

This should only affect dev environments with WP_DEBUG enabled, though, since they’re no more global as soon as compiled (there the IIFE is already in use).

Currently, I consider this a minor issue as it doesn’t effect production sites.

However, it’s already planned to refactor the JavaScript in version 2.0 since it’s a breaking change. So I added this to the list of #67.

Thank you for pointing out the problem. 🤗

@MatzeKitt MatzeKitt mentioned this issue Jul 7, 2023
6 tasks
@kraftner
Copy link
Contributor Author

kraftner commented Jul 7, 2023

Oh, yeah, of course. 🤦‍♂️ Good morning, sorry for the noise. 😀

@MatzeKitt
Copy link
Member

No problem, I didn’t have it on the list. Better this way than another. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants