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.

Sign up for GitHub

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

Jump to bottom

Update escaping function #665

Merged
merged 2 commits into from
Jun 3, 2024
Merged

Update escaping function #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1736824
update escaping function
matiasbenedetto Jun 3, 2024
9b223b7
simplify check to know if the text is already escaped.
matiasbenedetto Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions includes/create-theme/theme-locale.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ public static function escape_string( $string ) {
}

// Check if the text is already escaped.
if ( str_starts_with( $string, '<?php echo' ) ) {
if ( str_starts_with( $string, '<?php' ) ) {
return $string;
}

$string = addcslashes( $string, "'" );
return "<?php echo __('" . $string . "', '" . wp_get_theme()->get( 'TextDomain' ) . "');?>";
return "<?php esc_html_e('" . $string . "', '" . wp_get_theme()->get( 'TextDomain' ) . "');?>";
}

/**
Expand Down
10 changes: 5 additions & 5 deletions tests/CbtThemeLocale/escapeString.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,29 @@ class CBT_Theme_Locale_EscapeString extends CBT_Theme_Locale_UnitTestCase {
public function test_escape_string() {
$string = 'This is a test text.';
$escaped_string = CBT_Theme_Locale::escape_string( $string );
$this->assertEquals( "<?php echo __('This is a test text.', 'test-locale-theme');?>", $escaped_string );
$this->assertEquals( "<?php esc_html_e('This is a test text.', 'test-locale-theme');?>", $escaped_string );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the changes, but why are we using CamelCase for these test files? It'd be better if we kept the convention we're already using for the other files. Same for the directory name.

Copy link
Contributor Author

@matiasbenedetto matiasbenedetto Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the WordPress core convention I think we should follow it. Example: https://github.com/WordPress/wordpress-develop/tree/trunk/tests/phpunit/tests/blocks

Copy link
Contributor Author

@matiasbenedetto matiasbenedetto Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we kept the convention we're already using for the other files. Same for the directory name.

Those should be progressively updated when we have the chance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I submitted an issue for standardizing the convention used on tests: #666

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think those tests follow the actual convention either:

https://make.wordpress.org/docs/style-guide/formatting/filenames/#naming-files

Use lowercase file, folder, and directory names. In general, separate words in filenames with hyphens, not underscores. Use standard ASCII alphanumeric characters in file, folder, and directory names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use lowercase file, folder, and directory names.

I'm not sure because, as far as I saw, the are numerous core test files using camel case.

Example 2: https://github.com/WordPress/wordpress-develop/tree/trunk/tests/phpunit/tests/blocks
Example 1: https://github.com/WordPress/wordpress-develop/tree/trunk/tests/phpunit/tests/fonts/font-face

}

public function test_escape_string_with_single_quote() {
$string = "This is a test text with a single quote '";
$escaped_string = CBT_Theme_Locale::escape_string( $string );
$this->assertEquals( "<?php echo __('This is a test text with a single quote \\'', 'test-locale-theme');?>", $escaped_string );
$this->assertEquals( "<?php esc_html_e('This is a test text with a single quote \\'', 'test-locale-theme');?>", $escaped_string );
}

public function test_escape_string_with_double_quote() {
$string = 'This is a test text with a double quote "';
$escaped_string = CBT_Theme_Locale::escape_string( $string );
$this->assertEquals( "<?php echo __('This is a test text with a double quote \"', 'test-locale-theme');?>", $escaped_string );
$this->assertEquals( "<?php esc_html_e('This is a test text with a double quote \"', 'test-locale-theme');?>", $escaped_string );
}

public function test_escape_string_with_html() {
$string = '<p>This is a test text with HTML.</p>';
$escaped_string = CBT_Theme_Locale::escape_string( $string );
$this->assertEquals( "<?php echo __('<p>This is a test text with HTML.</p>', 'test-locale-theme');?>", $escaped_string );
$this->assertEquals( "<?php esc_html_e('<p>This is a test text with HTML.</p>', 'test-locale-theme');?>", $escaped_string );
}

public function test_escape_string_with_already_escaped_string() {
$string = "<?php echo __('This is a test text.', 'test-locale-theme');?>";
$string = "<?php esc_html_e('This is a test text.', 'test-locale-theme');?>";
$escaped_string = CBT_Theme_Locale::escape_string( $string );
$this->assertEquals( $string, $escaped_string );
}
Expand Down
32 changes: 16 additions & 16 deletions tests/CbtThemeLocale/escapeTextContentOfBlocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function data_test_escape_text_content_of_blocks() {

'paragraph' => array(
'block_markup' => '<!-- wp:paragraph {"align":"center"} --><p class="has-text-align-center">This is a test text.</p><!-- /wp:paragraph -->',
'expected_markup' => '<!-- wp:paragraph {"align":"center"} --><p class="has-text-align-center"><?php echo __(\'This is a test text.\', \'test-locale-theme\');?></p><!-- /wp:paragraph -->',
'expected_markup' => '<!-- wp:paragraph {"align":"center"} --><p class="has-text-align-center"><?php esc_html_e(\'This is a test text.\', \'test-locale-theme\');?></p><!-- /wp:paragraph -->',
),

'paragraph on nested groups' => array(
Expand All @@ -46,7 +46,7 @@ public function data_test_escape_text_content_of_blocks() {
'<!-- wp:group {"align":"full","style":{"spacing":{"padding":{"top":"var:preset|spacing|50","bottom":"var:preset|spacing|50","left":"var:preset|spacing|50","right":"var:preset|spacing|50"}}},"layout":{"type":"constrained","contentSize":"","wideSize":""}} -->
<div class="wp-block-group alignfull" style="padding-top:var(--wp--preset--spacing--50);padding-right:var(--wp--preset--spacing--50);padding-bottom:var(--wp--preset--spacing--50);padding-left:var(--wp--preset--spacing--50)"><!-- wp:group {"style":{"spacing":{"blockGap":"0px"}},"layout":{"type":"constrained","contentSize":"565px"}} -->
<div class="wp-block-group"><!-- wp:paragraph {"align":"center"} -->
<p class="has-text-align-center"><?php echo __(\'This is a test text.\', \'test-locale-theme\');?></p>
<p class="has-text-align-center"><?php esc_html_e(\'This is a test text.\', \'test-locale-theme\');?></p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->',
Expand All @@ -59,7 +59,7 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:heading -->',
'expected_markup' =>
'<!-- wp:heading {"textAlign":"center","className":"is-style-asterisk"} -->
<h1 class="wp-block-heading has-text-align-center is-style-asterisk"><?php echo __(\'A passion for creating spaces\', \'test-locale-theme\');?></h1>
<h1 class="wp-block-heading has-text-align-center is-style-asterisk"><?php esc_html_e(\'A passion for creating spaces\', \'test-locale-theme\');?></h1>
<!-- /wp:heading -->',
),

Expand All @@ -70,7 +70,7 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:heading -->',
'expected_markup' =>
'<!-- wp:heading {"textAlign":"center","className":"is-style-asterisk"} -->
<h2 class="wp-block-heading has-text-align-center is-style-asterisk"><?php echo __(\'A passion for creating spaces\', \'test-locale-theme\');?></h2>
<h2 class="wp-block-heading has-text-align-center is-style-asterisk"><?php esc_html_e(\'A passion for creating spaces\', \'test-locale-theme\');?></h2>
<!-- /wp:heading -->',
),

Expand All @@ -90,13 +90,13 @@ public function data_test_escape_text_content_of_blocks() {
'expected_markup' =>
'<!-- wp:list {"style":{"typography":{"lineHeight":"1.75"}},"className":"is-style-checkmark-list"} -->
<ul style="line-height:1.75" class="is-style-checkmark-list"><!-- wp:list-item -->
<li><?php echo __(\'Collaborate with fellow architects.\', \'test-locale-theme\');?></li>
<li><?php esc_html_e(\'Collaborate with fellow architects.\', \'test-locale-theme\');?></li>
<!-- /wp:list-item -->
<!-- wp:list-item -->
<li><?php echo __(\'Showcase your projects.\', \'test-locale-theme\');?></li>
<li><?php esc_html_e(\'Showcase your projects.\', \'test-locale-theme\');?></li>
<!-- /wp:list-item -->
<!-- wp:list-item -->
<li><?php echo __(\'Experience the world of architecture.\', \'test-locale-theme\');?></li>
<li><?php esc_html_e(\'Experience the world of architecture.\', \'test-locale-theme\');?></li>
<!-- /wp:list-item --></ul>
<!-- /wp:list -->',
),
Expand All @@ -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 echo __(\'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 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>
<!-- /wp:verse -->',
),

Expand All @@ -119,7 +119,7 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:button -->',
'expected_markup' =>
'<!-- wp:button -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button"><?php echo __(\'Sign up\', \'test-locale-theme\');?></a></div>
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button"><?php esc_html_e(\'Sign up\', \'test-locale-theme\');?></a></div>
<!-- /wp:button -->',
),

Expand All @@ -130,7 +130,7 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:image -->',
'expected_markup' =>
'<!-- wp:image {"sizeSlug":"large","linkDestination":"none","className":"is-style-rounded"} -->
<figure class="wp-block-image size-large is-style-rounded"><img src="http://localhost/wp1/wp-content/themes/twentytwentyfour/assets/images/windows.webp" alt="<?php echo __(\'Windows of a building in Nuremberg, Germany\', \'test-locale-theme\');?>"/></figure>
<figure class="wp-block-image size-large is-style-rounded"><img src="http://localhost/wp1/wp-content/themes/twentytwentyfour/assets/images/windows.webp" alt="<?php esc_html_e(\'Windows of a building in Nuremberg, Germany\', \'test-locale-theme\');?>"/></figure>
<!-- /wp:image -->',
),

Expand All @@ -143,8 +143,8 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:cover -->',
'expected_markup' =>
'<!-- wp:cover {"url":"http://localhost/wp1/wp-content/uploads/2024/05/image.jpeg","id":39,"alt":"Alternative text for cover image","dimRatio":50,"customOverlayColor":"#1d2b2f","layout":{"type":"constrained"}} -->
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim" style="background-color:#1d2b2f"></span><img class="wp-block-cover__image-background wp-image-39" alt="<?php echo __(\'Alternative text for cover image\', \'test-locale-theme\');?>" src="http://localhost/wp1/wp-content/uploads/2024/05/image.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size"><?php echo __(\'This is a cover caption\', \'test-locale-theme\');?></p>
<div class="wp-block-cover"><span aria-hidden="true" class="wp-block-cover__background has-background-dim" style="background-color:#1d2b2f"></span><img class="wp-block-cover__image-background wp-image-39" alt="<?php esc_html_e(\'Alternative text for cover image\', \'test-locale-theme\');?>" src="http://localhost/wp1/wp-content/uploads/2024/05/image.jpeg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size"><?php esc_html_e(\'This is a cover caption\', \'test-locale-theme\');?></p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->',
),
Expand All @@ -158,8 +158,8 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:media-text -->',
'expected_markup' =>
'<!-- wp:media-text {"mediaId":39,"mediaLink":"http://localhost/wp1/image/","mediaType":"image"} -->
<div class="wp-block-media-text is-stacked-on-mobile"><figure class="wp-block-media-text__media"><img src="http://localhost/wp1/wp-content/uploads/2024/05/image.jpeg" alt="<?php echo __(\'This is alt text\', \'test-locale-theme\');?>" class="wp-image-39 size-full"/></figure><div class="wp-block-media-text__content"><!-- wp:paragraph {"placeholder":"Content…"} -->
<p><?php echo __(\'Media text content test.\', \'test-locale-theme\');?></p>
<div class="wp-block-media-text is-stacked-on-mobile"><figure class="wp-block-media-text__media"><img src="http://localhost/wp1/wp-content/uploads/2024/05/image.jpeg" alt="<?php esc_html_e(\'This is alt text\', \'test-locale-theme\');?>" class="wp-image-39 size-full"/></figure><div class="wp-block-media-text__content"><!-- wp:paragraph {"placeholder":"Content…"} -->
<p><?php esc_html_e(\'Media text content test.\', \'test-locale-theme\');?></p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:media-text -->',
),
Expand All @@ -171,7 +171,7 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:pullquote -->',
'expected_markup' =>
'<!-- wp:pullquote -->
<figure class="wp-block-pullquote"><blockquote><p><?php echo __(\'Yo me equivoqué y pagué, pero la pelota no se mancha.\', \'test-locale-theme\');?></p><cite><?php echo __(\'Diego Armando Maradona\', \'test-locale-theme\');?></cite></blockquote></figure>
<figure class="wp-block-pullquote"><blockquote><p><?php esc_html_e(\'Yo me equivoqué y pagué, pero la pelota no se mancha.\', \'test-locale-theme\');?></p><cite><?php esc_html_e(\'Diego Armando Maradona\', \'test-locale-theme\');?></cite></blockquote></figure>
<!-- /wp:pullquote -->',
),

Expand All @@ -182,7 +182,7 @@ public function data_test_escape_text_content_of_blocks() {
<!-- /wp:table -->',
'expected_markup' =>
'<!-- wp:table -->
<figure class="wp-block-table"><table><tbody><tr><td><?php echo __(\'Team\', \'test-locale-theme\');?></td><td><?php echo __(\'Points\', \'test-locale-theme\');?></td></tr><tr><td><?php echo __(\'Boca\', \'test-locale-theme\');?></td><td><?php echo __(\'74\', \'test-locale-theme\');?></td></tr><tr><td><?php echo __(\'River\', \'test-locale-theme\');?></td><td><?php echo __(\'2\', \'test-locale-theme\');?></td></tr></tbody></table><figcaption class="wp-element-caption"><?php echo __(\'Score table\', \'test-locale-theme\');?></figcaption></figure>
<figure class="wp-block-table"><table><tbody><tr><td><?php esc_html_e(\'Team\', \'test-locale-theme\');?></td><td><?php esc_html_e(\'Points\', \'test-locale-theme\');?></td></tr><tr><td><?php esc_html_e(\'Boca\', \'test-locale-theme\');?></td><td><?php esc_html_e(\'74\', \'test-locale-theme\');?></td></tr><tr><td><?php esc_html_e(\'River\', \'test-locale-theme\');?></td><td><?php esc_html_e(\'2\', \'test-locale-theme\');?></td></tr></tbody></table><figcaption class="wp-element-caption"><?php esc_html_e(\'Score table\', \'test-locale-theme\');?></figcaption></figure>
<!-- /wp:table -->',
),

Expand Down
Loading
Loading