Skip to content

Comments

Adding OutputFilter::stringJSSafe()#56

Closed
Hackwar wants to merge 1 commit into3.x-devfrom
3.x-jssafe
Closed

Adding OutputFilter::stringJSSafe()#56
Hackwar wants to merge 1 commit into3.x-devfrom
3.x-jssafe

Conversation

@Hackwar
Copy link
Contributor

@Hackwar Hackwar commented Apr 19, 2023

Summary of Changes

This is part of the process of deprecating the CMS Filter package in favor of the framework package. This adds the stringJSSafe() method to the OutputFilter class.

Testing Instructions

Tests should pass. Codereview.

Documentation Changes Required

@Llewellynvdm
Copy link

The provided PHP code is a valid solution to escape a string for use in JavaScript. However, there's a more efficient and optimal solution using the json_encode function. The json_encode function will not only escape the string but will also take care of encoding it in a format compatible with JavaScript.

Here's an alternative solution:

/**
 * Processes a string and escapes it for use in JavaScript
 *
 * @param   string  $string  String to process
 *
 * @return  string  Processed text
 *
 * @since   3.0
 */
public static function stringJSSafe(string $string): string
{
    return json_encode($string, JSON_UNESCAPED_UNICODE);
}

This function simply calls json_encode with the JSON_UNESCAPED_UNICODE flag, which encodes the string in a format compatible with JavaScript, without escaping Unicode characters. This is more efficient than the original solution because it avoids manual string manipulation and uses a built-in PHP function that is designed for this purpose.

Here is what I understand would motivate the current solution given:

  1. Customization: The original solution allows you to customize the encoding process to meet specific requirements. For example, you might need to escape certain characters in a particular way, or to handle special cases that json_encode doesn't cover.
  2. Explicit Unicode escaping: The original solution explicitly escapes all Unicode characters using the \uXXXX notation or \u{XXXXX} for characters with code points above 0xFFFF. This can result in a larger string size, but it ensures that the output will only contain ASCII characters. This can be useful in situations where the target JavaScript environment has limited Unicode support, or when you want to guarantee that the output will not contain any non-ASCII characters.
  3. Educational value: Implementing your own string escaping function can help you better understand Unicode and character encoding concepts. It can be a valuable learning experience, especially for developers who are new to PHP or character encoding.

That said, using json_encode with the JSON_UNESCAPED_UNICODE flag is generally the preferred method because it is more efficient, less error-prone, and easier to maintain.

So I am just wondering what is the real issue here that prevent us from using json_encode

@Hackwar
Copy link
Contributor Author

Hackwar commented Apr 19, 2023

I've copied over the code from the CMS. The goal is to remove the duplicate code between framework and CMS and making the filter package 100% compatible with the CMS classes is a prerequisite to convert over the Input package. I don't know if it would be B/C to change this to json_encode and to be on the safe side, I would keep the code as is for now.

@Hackwar
Copy link
Contributor Author

Hackwar commented Apr 24, 2023

I'm closing this in favour of this being rebased to 2.0-dev to hopefully add this to Joomla 4.4 already.

@Hackwar Hackwar closed this Apr 24, 2023
@Hackwar Hackwar deleted the 3.x-jssafe branch April 24, 2023 12:18
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

Successfully merging this pull request may close these issues.

2 participants