Process inner html of blocks when escaping text content (#719)

* WIP working to convert attributes and URLs. * Ensure media is added to local. * Use a list of tokens and sprintf to generate the formatted string. * Only format the string if tokens are present. * Update content with html test. Update remaining tests with inner markup. Try and fix tests. Format tests. * Remove whitespace. * Process all attributes Escape at the end. Provide better translation note. * Refactor token processing to its own class. * Update tests with string replacements and translation. * Check if % exists in the text and escape it. * Add a test case for a localizing text that includes a %. * Add new line at the end of translators note * Reformat numbers in translators note * Update test * Move new line to theme-locale * Handle self closing tags * Refactor how new line is added * Attempt to fix tests * Attempt to fix tests again --------- Co-authored-by: Grant Kinney <1699996+creativecoder@users.noreply.github.com> Co-authored-by: Sarah Norris <sarah@sekai.co.uk> Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com>
WordPress · Oct 23, 2024 · 512b9f8 · 512b9f8
1 parent 33c5ff9
commit 512b9f8
Show file tree

Hide file tree

Showing 5 changed files with 182 additions and 9 deletions.
diff --git a/includes/create-theme/theme-locale.php b/includes/create-theme/theme-locale.php
@@ -2,6 +2,9 @@
 /*
 * Locale related functionality
 */
+
+require_once __DIR__ . '/theme-token-processor.php';
+
 class CBT_Theme_Locale {
 
 	/**
@@ -28,6 +31,27 @@ private static function escape_text_content( $string ) {
 
 		$string = addcslashes( $string, "'" );
 
+		$p = new CBT_Token_Processor( $string );
+		$p->process_tokens();
+		$text             = $p->get_text();
+		$tokens           = $p->get_tokens();
+		$translators_note = $p->get_translators_note();
+
+		if ( ! empty( $tokens ) ) {
+			$php_tag  = '<?php ';
+			$php_tag .= $translators_note . "\n";
+			$php_tag .= "echo sprintf( esc_html__( '$text', '" . wp_get_theme()->get( 'TextDomain' ) . "' ), " . implode(
+				', ',
+				array_map(
+					function( $token ) {
+						return "'$token'";
+					},
+					$tokens
+				)
+			) . ' ); ?>';
+			return $php_tag;
+		}
+
 		return "<?php esc_html_e('" . $string . "', '" . wp_get_theme()->get( 'TextDomain' ) . "');?>";
 	}
 

diff --git a/includes/create-theme/theme-token-processor.php b/includes/create-theme/theme-token-processor.php
@@ -0,0 +1,134 @@
+<?php
+/**
+ * Fine grained token processing class.
+ */
+class CBT_Token_Processor {
+	private $p;
+	private $tokens           = array();
+	private $text             = '';
+	private $translators_note = '/* Translators: ';
+	private $increment        = 0;
+
+	/**
+	 * Constructor.
+	 *
+	 * @param string $string The string to process.
+	 */
+	public function __construct( $string ) {
+		$this->p = new WP_HTML_Tag_Processor( $string );
+	}
+
+	/**
+	 * Processes the HTML tags in the string and updates tokens, text, and translators' note.
+	 *
+	 * @param $p The string to process.
+	 * @return void
+	 */
+	public function process_tokens() {
+		while ( $this->p->next_token() ) {
+			$token_type      = $this->p->get_token_type();
+			$token_name      = strtolower( $this->p->get_token_name() );
+			$is_tag_closer   = $this->p->is_tag_closer();
+			$has_self_closer = $this->p->has_self_closing_flag();
+
+			if ( '#tag' === $token_type ) {
+				$this->increment++;
+				$this->text .= '%' . $this->increment . '$s';
+				$token_label = $this->increment . '.';
+
+				if ( 1 !== $this->increment ) {
+					$this->translators_note .= ', ';
+				}
+
+				if ( $is_tag_closer ) {
+					$this->tokens[]          = "</{$token_name}>";
+					$this->translators_note .= $token_label . " is the end of a '" . $token_name . "' HTML element";
+				} else {
+					$token      = '<' . $token_name;
+					$attributes = $this->p->get_attribute_names_with_prefix( '' );
+
+					foreach ( $attributes as $attr_name ) {
+						$attr_value = $this->p->get_attribute( $attr_name );
+						$token     .= $this->process_attribute( $attr_name, $attr_value );
+					}
+
+					$token         .= '>';
+					$this->tokens[] = $token;
+
+					if ( $has_self_closer || 'br' === $token_name ) {
+						$this->translators_note .= $token_label . " is a '" . $token_name . "' HTML element";
+					} else {
+						$this->translators_note .= $token_label . " is the start of a '" . $token_name . "' HTML element";
+					}
+				}
+			} else {
+				// Escape text content.
+				$temp_text = $this->p->get_modifiable_text();
+
+				// If the text contains a %, we need to escape it.
+				if ( false !== strpos( $temp_text, '%' ) ) {
+					$temp_text = str_replace( '%', '%%', $temp_text );
+				}
+
+				$this->text .= $temp_text;
+			}
+		}
+
+		if ( ! empty( $this->tokens ) ) {
+			$this->translators_note .= ' */ ';
+		}
+	}
+
+	/**
+	 * Processes individual tag attributes and escapes where necessary.
+	 *
+	 * @param string $attr_name The name of the attribute.
+	 * @param string $attr_value The value of the attribute.
+	 * @return string The processed attribute.
+	 */
+	private function process_attribute( $attr_name, $attr_value ) {
+		$token_part = '';
+		if ( empty( $attr_value ) ) {
+			$token_part .= ' ' . $attr_name;
+		} elseif ( 'src' === $attr_name ) {
+			CBT_Theme_Media::add_media_to_local( array( $attr_value ) );
+			$relative_src = CBT_Theme_Media::get_media_folder_path_from_url( $attr_value ) . basename( $attr_value );
+			$attr_value   = "' . esc_url( get_stylesheet_directory_uri() ) . '{$relative_src}";
+			$token_part  .= ' ' . $attr_name . '="' . $attr_value . '"';
+		} elseif ( 'href' === $attr_name ) {
+			$attr_value  = "' . esc_url( '$attr_value' ) . '";
+			$token_part .= ' ' . $attr_name . '="' . $attr_value . '"';
+		} else {
+			$token_part .= ' ' . $attr_name . '="' . $attr_value . '"';
+		}
+
+		return $token_part;
+	}
+
+	/**
+	 * Gets the processed text.
+	 *
+	 * @return string
+	 */
+	public function get_text() {
+		return $this->text;
+	}
+
+	/**
+	 * Gets the processed tokens.
+	 *
+	 * @return array
+	 */
+	public function get_tokens() {
+		return $this->tokens;
+	}
+
+	/**
+	 * Gets the generated translators' note.
+	 *
+	 * @return string
+	 */
+	public function get_translators_note() {
+		return $this->translators_note;
+	}
+}
diff --git a/tests/CbtThemeLocale/escapeTextContent.php b/tests/CbtThemeLocale/escapeTextContent.php
@@ -37,9 +37,10 @@ public function test_escape_text_content_with_double_quote() {
 	}
 
 	public function test_escape_text_content_with_html() {
-		$string         = '<p>This is a test text with HTML.</p>';
-		$escaped_string = $this->call_private_method( 'escape_text_content', array( $string ) );
-		$this->assertEquals( "<?php esc_html_e('<p>This is a test text with HTML.</p>', 'test-locale-theme');?>", $escaped_string );
+		$string          = '<p>This is a test text with HTML.</p>';
+		$escaped_string  = $this->call_private_method( 'escape_text_content', array( $string ) );
+		$expected_output = '<?php /* Translators: 1. is the start of a \'p\' HTML element, 2. is the end of a \'p\' HTML element */' . " \n" . 'echo sprintf( esc_html__( \'%1$sThis is a test text with HTML.%2$s\', \'test-locale-theme\' ), \'<p>\', \'</p>\' ); ?>';
+		$this->assertEquals( $expected_output, $escaped_string );
 	}
 
 	public function test_escape_text_content_with_already_escaped_string() {

diff --git a/tests/CbtThemeLocale/escapeTextContentOfBlocks.php b/tests/CbtThemeLocale/escapeTextContentOfBlocks.php
@@ -108,7 +108,7 @@ public function data_test_escape_text_content_of_blocks() {
                     <!-- /wp:verse -->',
 				'expected_markup' =>
 					'<!-- wp:verse {"style":{"layout":{"selfStretch":"fit","flexSize":null}}} -->
-                    <pre class="wp-block-verse"><?php esc_html_e(\'Ya somos el olvido que seremos.<br>El polvo elemental que nos ignora<br>y que fue el rojo Adán y que es ahora<br>todos los hombres, y que no veremos.\', \'test-locale-theme\');?></pre>
+                    <pre class="wp-block-verse"><?php /* Translators: 1. is a \'br\' HTML element, 2. is a \'br\' HTML element, 3. is a \'br\' HTML element */ ' . "\n" . 'echo sprintf( esc_html__( \'Ya somos el olvido que seremos.%1$sEl polvo elemental que nos ignora%2$sy que fue el rojo Adán y que es ahora%3$stodos los hombres, y que no veremos.\', \'test-locale-theme\' ), \'<br>\', \'<br>\', \'<br>\' ); ?></pre>
                     <!-- /wp:verse -->',
 			),
 

diff --git a/tests/test-theme-templates.php b/tests/test-theme-templates.php
@@ -159,12 +159,12 @@ public function test_properly_encode_lessthan_and_greaterthan() {
 
 	public function test_properly_encode_html_markup() {
 		$template          = new stdClass();
-		$template->content = '<!-- wp:paragraph -->
-			<p><strong>Bold</strong> text has feelings &lt;&gt; TOO</p>
-			<!-- /wp:paragraph -->';
+		$template->content = '<!-- wp:paragraph --><p><strong>Bold</strong> text has feelings &lt;&gt; TOO</p><!-- /wp:paragraph -->';
 		$escaped_template  = CBT_Theme_Templates::escape_text_in_template( $template );
 
-		$this->assertStringContainsString( "<?php esc_html_e('<strong>Bold</strong> text has feelings &lt;&gt; TOO', '');?>", $escaped_template->content );
+		$expected_output = '<!-- wp:paragraph --><p><?php /* Translators: 1. is the start of a \'strong\' HTML element, 2. is the end of a \'strong\' HTML element */ ' . "\n" . 'echo sprintf( esc_html__( \'%1$sBold%2$s text has feelings <> TOO\', \'\' ), \'<strong>\', \'</strong>\' ); ?></p><!-- /wp:paragraph -->';
+
+		$this->assertStringContainsString( $expected_output, $escaped_template->content );
 	}
 
 	public function test_empty_alt_text_is_not_localized() {
@@ -262,7 +262,21 @@ public function test_localize_verse() {
 			<pre class="wp-block-verse">Here is some <strong>verse</strong> to localize</pre>
 		<!-- /wp:verse -->';
 		$new_template      = CBT_Theme_Templates::escape_text_in_template( $template );
-		$this->assertStringContainsString( "<?php esc_html_e('Here is some <strong>verse</strong> to localize', '');?>", $new_template->content );
+
+		$expected_output = '<!-- wp:verse -->
+			<pre class="wp-block-verse"><?php /* Translators: 1. is the start of a \'strong\' HTML element, 2. is the end of a \'strong\' HTML element */ ' . "\n" . 'echo sprintf( esc_html__( \'Here is some %1$sverse%2$s to localize\', \'\' ), \'<strong>\', \'</strong>\' ); ?></pre>
+		<!-- /wp:verse -->';
+
+		$this->assertStringContainsString( $expected_output, $new_template->content );
+	}
+
+	public function test_localize_text_with_placeholders() {
+		$template          = new stdClass();
+		$template->content = '<!-- wp:paragraph -->
+			<p>This is <strong>bold text</strong> with a %s placeholder</p>
+		<!-- /wp:paragraph -->';
+		$new_template      = CBT_Theme_Templates::escape_text_in_template( $template );
+		$this->assertStringContainsString( '<?php /* Translators: 1. is the start of a \'strong\' HTML element, 2. is the end of a \'strong\' HTML element */ ' . "\n" . 'echo sprintf( esc_html__( \'This is %1$sbold text%2$s with a %%s placeholder\', \'\' ), \'<strong>\', \'</strong>\' ); ?>', $new_template->content );
 	}
 
 	public function test_localize_table() {