Smarty html_checkboxes_ex html_radios_ex を廃止

[seasoftjapan]

コメントの内容を踏まえ、以下を PR する予定。

• テンプレートで html_radios_ex を利用している箇所は、html_radios に置換する。
• html_radios_ex は以前のバージョンのテンプレートが使用されることを考慮して残すが、内部的には html_radios を呼び出す。その上で、deprecated とする。
• html_checkboxes_ex は削除する。
• phpstan.neon.dist で、ignoreErrors していた箇所を削除する。

多分、#800 と競合する。

[seasoftjapan]

html_checkboxes や html_radios を拡張している理由って何でしょうか?

PHPStan で、
function.html_checkboxes_ex.php や
function.html_radios_ex.php が引っかかって、実際に幾らか疑義がある実装と見受けます。

ざっと、Smarty の標準実装と比べると、label_ids が false true に変わっているようですが、他にも多数の差がありそうです。
標準実装の変更を追従していないと思うので、危なっかしさも感じます。

そもそも拡張が必要かや、現実装が実現方法として望ましいかも気になりますが、少なくとも必要となった理由はソースに記録しておくべきと思い起票した次第です。

[nanasess]

EC-CUBE2.13から2.17へバージョンアップする際に、2.13のテンプレートとの互換性を維持するためにこのような実装にしたと記憶しています。
Smarty3 以降の html_checkboxes や html_radios でも互換性を吸収できるようでしたら、その方がよさそうです

[seasoftjapan]

ちょっと勘違いがありました。Smarty 標準のカスタム関数を書き換えていると思っていたのですが、新たなカスタム関数として *_ex が追加されている様子ですね。

html_radios_ex の利用箇所

• 現在は以下の1箇所で使われている。

data/Smarty/templates/admin/products/review_edit.tpl:                <td><!--{html_radios_ex name="sex" options=$arrSex selected=$arrForm.sex}--></td>

• 2.4系以前では、アンケート機能 data/Smarty/templates/admin/contents/inquiry.tpl でも使われていた。

html_checkboxes_ex の利用箇所

• 現在は利用していない。
• 2.4系以前に使われていた。

よって、html_checkboxes_ex は直ちに削除で良さそうですが、html_radios_ex はもう少し調査が必要そうです。

[nobuhiko]

古いSmartyは実装がいまいちで、labelとか使ってないから拡張していたイメージがあります

[seasoftjapan]

function.html_radios_ex.php が追加された時点のソースで差異を調べました。

$ git show 7ea6ec8e9:data/module/Smarty/libs/plugins/function.html_radios.php | perl -pe 's/ +$//g' > 7ea6ec8e9-function.html_radios.php
$ git show 7ea6ec8e9:data/smarty_extends/function.html_radios_ex.php | perl -pe 's/ +$//g' > 7ea6ec8e9-function.html_radios_ex.php
$ diff -w -u 7ea6ec8e9-function.html_radios.php 7ea6ec8e9-function.html_radios_ex.php

--- 7ea6ec8e9-function.html_radios.php  2024-01-06 17:57:52.115876226 +0900
+++ 7ea6ec8e9-function.html_radios_ex.php       2024-01-06 17:58:03.855873704 +0900
@@ -38,7 +38,7 @@
  * @return string
  * @uses smarty_function_escape_special_chars()
  */
-function smarty_function_html_radios($params, &$smarty)
+function smarty_function_html_radios_ex($params, &$smarty)
 {
     require_once $smarty->_get_plugin_filepath('shared','escape_special_chars');

@@ -48,12 +48,15 @@
     $selected = null;
     $separator = '';
     $labels = true;
-    $label_ids = false;
+    $label_ids = true;
     $output = null;
     $extra = '';

     foreach($params as $_key => $_val) {
         switch($_key) {
+                       case 'tags':
+                               $$_key = split("\|", $_val);
+                               break;
             case 'name':
             case 'separator':
                 $$_key = (string)$_val;
@@ -76,7 +79,6 @@
             case 'options':
                 $$_key = (array)$_val;
                 break;
-
             case 'values':
             case 'output':
                 $$_key = array_values((array)$_val);
@@ -108,13 +110,13 @@
     if (isset($options)) {

         foreach ($options as $_key=>$_val)
-            $_html_result[] = smarty_function_html_radios_output($name, $_key, $_val, $selected, $extra, $separator, $labels, $label_ids);
+            $_html_result[] = smarty_function_html_radios_output_ex($name, $_key, $_val, $selected, $extra, $separator, $labels, $label_ids, $tags);

     } else {

         foreach ($values as $_i=>$_key) {
             $_val = isset($output[$_i]) ? $output[$_i] : '';
-            $_html_result[] = smarty_function_html_radios_output($name, $_key, $_val, $selected, $extra, $separator, $labels, $label_ids);
+            $_html_result[] = smarty_function_html_radios_output_ex($name, $_key, $_val, $selected, $extra, $separator, $labels, $label_ids, $tags);
         }

     }
@@ -127,8 +129,25 @@

 }

-function smarty_function_html_radios_output($name, $value, $output, $selected, $extra, $separator, $labels, $label_ids) {
+function smarty_function_html_radios_output_ex($name, $value, $output, $selected, $extra, $separator, $labels, $label_ids, $tags) {
     $_output = '';
+
+   $_output .= '<input type="radio" name="'
+        . smarty_function_escape_special_chars($name) . '" value="'
+        . smarty_function_escape_special_chars($value) . '"';
+
+   if ($labels && $label_ids) {
+          $_id = smarty_function_escape_special_chars(preg_replace('![^\w\-\.]!', '_', $name . '_' . $value));
+          $_output .= ' id="' . $_id . '"';
+   }
+    if ((string)$value == $selected) {
+        $_output .= ' checked="checked"';
+    }
+
+       $_output .= $extra . ' />';
+
+       $_output .= $tags[0];
+
     if ($labels) {
       if($label_ids) {
           $_id = smarty_function_escape_special_chars(preg_replace('![^\w\-\.]!', '_', $name . '_' . $value));
@@ -137,16 +156,12 @@
           $_output .= '<label>';
       }
    }
-   $_output .= '<input type="radio" name="'
-        . smarty_function_escape_special_chars($name) . '" value="'
-        . smarty_function_escape_special_chars($value) . '"';

-   if ($labels && $label_ids) $_output .= ' id="' . $_id . '"';
+       // �ͤ�����
+       $_output .= $output;
+
+       $_output .= $tags[1];

-    if ((string)$value==$selected) {
-        $_output .= ' checked="checked"';
-    }
-    $_output .= $extra . ' />' . $output;
     if ($labels) $_output .= '</label>';
     $_output .=  $separator;

ざっくり、以下の感じでしょうか。

• タグの順序を
  <label><input>{テキスト}</label> から
  <input><label>{テキスト}</label> に変えた。
• 属性 label_ids のデフォルトを false から true に変えた。おそらく本質的な目的ではなく、前項の変更により <label for=...> の記述が必須となるため。
• 属性 tags を追加した。

[seasoftjapan]

引き続き、EC-CUBE 2.11 以降で、唯一の利用箇所について考察します。

ec-cube2/data/Smarty/templates/admin/products/review_edit.tpl

Line 72 in c438132

<td><!--{html_radios_ex name="sex" options=$arrSex selected=$arrForm.sex}--></td>

• 属性 tags は利用していない。
• 同テンプレート上の「レビュー表示」もラジオボタンだが、ソースベタ書きである。<label> を記述していない。<input id=...> も記述していない。

{html_radios_ex} を {html_radios} に書き換えると...

 <td>
-<input type="radio" name="sex" value="1" id="sex_1"><label for="sex_1">男性</label>
+<label><input type="radio" name="sex" value="1">男性</label>
-<input type="radio" name="sex" value="2" id="sex_2"><label for="sex_2">女性</label>
+<label><input type="radio" name="sex" value="2">女性</label>
 </td>

• 画面操作すると、特に問題なく登録された。
• sex_1 及び sex_2 は、他で参照されていない。(Chrome 開発ツールの検索で確認した。)

Smarty 公式マニュアル のサンプルだと、id 値が出力される記述なので、その相違はやや気になりますが、実質的にはどちらでも良く、むしろ ID 競合のリスクが軽減するとも思います。

うろ覚えですが、大昔に <label><input>{テキスト}</label> で id, for 無しだと、クリッカブルにならないブラウザー・バージョンが存在した記憶がありますので、その対策がだったのかもしれませんね。

追って対応を考えて PR しようと思いますが、他に関連する情報がありましたら、引き続き教えてください。