From 68c147a4916b507037457f4b264ba46a6adfeb3c Mon Sep 17 00:00:00 2001
From: Corey McKrill <916023+coreymckrill@users.noreply.github.com>
Date: Thu, 4 Jan 2024 16:08:31 -0800
Subject: [PATCH] Settings: Make minor corrections to the HTML markup of
 settings fields (#43166)

* Corrections to HTML markup of settings controls

* Remove deprecated `valign` attributes from table row elements
* Remove orphaned quotation marks from the end of table row elements

Co-authored-by: Sagehen Studio <2924107+sagehenstudio@users.noreply.github.com>

* Enable title prop for `info` setting type

This gives the `<th>` for the info setting type a closing tag
since its not self-closing. It also allows for adding a `title` prop
to the info setting type, to match all the other setting types.

* Add changelog file

* phpcs cleanup

---------

Co-authored-by: Sagehen Studio <2924107+sagehenstudio@users.noreply.github.com>
---
 .../changelog/remix-41077-settings-html-fixes |  4 ++
 .../admin/class-wc-admin-settings.php         | 42 ++++++++++++-------
 2 files changed, 31 insertions(+), 15 deletions(-)
 create mode 100644 plugins/woocommerce/changelog/remix-41077-settings-html-fixes

diff --git a/plugins/woocommerce/changelog/remix-41077-settings-html-fixes b/plugins/woocommerce/changelog/remix-41077-settings-html-fixes
new file mode 100644
index 00000000000..abeadae0e22
--- /dev/null
+++ b/plugins/woocommerce/changelog/remix-41077-settings-html-fixes
@@ -0,0 +1,4 @@
+Significance: minor
+Type: update
+
+Make minor corrections to the HTML markup of settings fields
diff --git a/plugins/woocommerce/includes/admin/class-wc-admin-settings.php b/plugins/woocommerce/includes/admin/class-wc-admin-settings.php
index a0ec737087e..a322828e5b2 100644
--- a/plugins/woocommerce/includes/admin/class-wc-admin-settings.php
+++ b/plugins/woocommerce/includes/admin/class-wc-admin-settings.php
@@ -253,6 +253,11 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 					$value['value'] = self::get_option( $value['id'], $value['default'] );
 				}
 
+				$row_class_attribute = '';
+				if ( $value['row_class'] ) {
+					$row_class_attribute = " class=\"{$value['row_class']}\"";
+				}
+
 				// Custom attribute handling.
 				$custom_attributes = array();
 
@@ -287,8 +292,9 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						break;
 
 					case 'info':
-						?><tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
-							<th scope="row" class="titledesc"/><td style="<?php echo esc_attr( $value['css'] ); ?>">
+						?><tr<?php echo esc_html( $row_class_attribute ); ?>>
+							<th scope="row" class="titledesc"><?php echo esc_html( $value['title'] ); ?></th>
+							<td style="<?php echo esc_attr( $value['css'] ); ?>">
 						<?php
 						echo wp_kses_post( wpautop( wptexturize( $value['text'] ) ) );
 						echo '</td></tr>';
@@ -320,7 +326,8 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 					case 'tel':
 						$option_value = $value['value'];
 
-						?><tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						?>
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -345,7 +352,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						$option_value = $value['value'];
 
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -373,7 +380,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						$option_value = $value['value'];
 
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -399,7 +406,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						$option_value = $value['value'];
 
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -442,7 +449,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						$show_desc_at_end = $value['desc_at_end'] ?? false;
 
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -509,13 +516,18 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 							$visibility_class[] = $value['row_class'];
 						}
 
+						$container_class_attribute = '';
+						if ( count( $visibility_class ) > 0 ) {
+							$container_class_attribute = ' class="' . implode( ' ', $visibility_class ) . '"';
+						}
+
 						$must_disable = $value['disabled'] ?? false;
 
 						if ( ! isset( $value['checkboxgroup'] ) || 'start' === $value['checkboxgroup'] ) {
 							$has_tooltip             = isset( $value['tooltip'] ) && '' !== $value['tooltip'];
 							$tooltip_container_class = $has_tooltip ? 'with-tooltip' : '';
 							?>
-								<tr valign="top" class="<?php echo esc_attr( implode( ' ', $visibility_class ) ); ?>">
+								<tr<?php echo esc_html( $container_class_attribute ); ?>>
 									<th scope="row" class="titledesc"><?php echo esc_html( $value['title'] ); ?></th>
 									<td class="forminp forminp-checkbox <?php echo esc_html( $tooltip_container_class ); ?>">
 										<?php if ( $has_tooltip ) : ?>
@@ -525,7 +537,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 							<?php
 						} else {
 							?>
-								<fieldset class="<?php echo esc_attr( implode( ' ', $visibility_class ) ); ?>">
+								<fieldset<?php echo esc_html( $container_class_attribute ); ?>>
 							<?php
 						}
 
@@ -580,7 +592,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						}
 
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 							<label><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html . $disabled_message; // WPCS: XSS ok. ?></label>
 						</th>
@@ -614,7 +626,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						}
 
 						?>
-						<tr valign="top" class="single_select_page"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr class="single_select_page <?php echo esc_attr( $value['row_class'] ); ?>">
 							<th scope="row" class="titledesc">
 								<label><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -639,7 +651,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 							);
 						}
 						?>
-						<tr valign="top" class="single_select_page"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr class="single_select_page <?php echo esc_attr( $value['row_class'] ); ?>">
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?></label>
 							</th>
@@ -679,7 +691,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 							$state   = '*';
 						}
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -703,7 +715,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 
 						asort( $countries );
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>
@@ -732,7 +744,7 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
 						);
 						$option_value = wc_parse_relative_date_option( $value['value'] );
 						?>
-						<tr valign="top"<?php echo $value['row_class'] ? ' class="' . esc_attr( $value['row_class'] ) . '"' : '' ?>">
+						<tr<?php echo esc_html( $row_class_attribute ); ?>>
 							<th scope="row" class="titledesc">
 								<label for="<?php echo esc_attr( $value['id'] ); ?>"><?php echo esc_html( $value['title'] ); ?> <?php echo $tooltip_html; // WPCS: XSS ok. ?></label>
 							</th>