BugTrack/2279

calendar_viewer.inc.php と comment.inc.php の併用で XHTML 1.1 invalid†

ページ: BugTrack2
投稿者: Makichan
優先順位: 普通
状態: 提案
カテゴリー: プラグイン
投稿日: 2007-10-11 (木) 10:54:18
バージョン:

メッセージ†

calendar_viewer で表示した複数の記事中に comment プラグインが使われていると、フォーム中の "msg" および "name" に設定された id が重複してしまい、XHTML 1.1 invalid になってしまう。

calendar_viewer.inc.php と comment.inc.php の併用で XHTML 1.1 invalid
- メッセージ
- コメント

↑

問題点†

calendar_viewer.inc.php 160行付近

	// $limit_page の件数までインクルード
	$tmp = max($limit_base, 0); // Skip minus
	while ($tmp < $limit_page) {
		if (! isset($pagelist[$tmp])) break;

		$page = $pagelist[$tmp];
		$get['page'] = $post['page'] = $vars['page'] = $page;

$vars['page'] にインクルードする記事名をセットしている。

comment.inc.php 90行付近

function plugin_comment_convert()
{
	global $vars, $digest, $_btn_comment, $_btn_name, $_msg_comment;
	static $numbers = array();
	static $comment_cols = PLUGIN_COMMENT_SIZE_MSG;

	if (PKWK_READONLY) return ''; // Show nothing

	if (! isset($numbers[$vars['page']])) $numbers[$vars['page']] = 0;
	$comment_no = $numbers[$vars['page']]++;

$vars['page'] をキーにして$comment_noをセットしている。
$comment_no はその後 id の生成に利用している。

comment.inc.php の挙動は、同一ページ内に複数の #comment があっても id が被らないようにしている。
calendar_viewer を使用すると、親ページ内に複数の子ページが存在するイメージとなり、子ページ内での整合性は保たれるものの、子ページ間での競合が避けられない。

↑

問題点の本質†

label要素(に限らず、(x)htmlすべて)におけるid属性は、(x)html文書内で一意で無ければならない。
calendar_viewer.inc.phpは1つの(x)html文書内に複数のページ(wiki)を表示しようとする。
comment.inc.phpにおける$comment_no変数は、(x)html文書内での表示位置/順序に係わらず、ページ(wiki)内での出現順で普遍の一意の値で無ければならない。
管理の基準となる対象が違う二つの要素を一つの変数で管理しようとしているところに問題がある。

↑

改修案1†

input要素中の id は、label要素との紐付けのためだけに使われており、プログラム的には何ら意味の無い値である。
commentプラグインにて、idの生成にはページ名に拠らず出現回数のみを拠り所にする変数を新設して、それを使うように修正することでこの問題は解決できる。

comment.inc.php

function plugin_comment_convert()
 {
 	global $vars, $digest, $_btn_comment, $_btn_name, $_msg_comment;
 	static $numbers = array();
+	static $commentnums = 0;
 	static $comment_cols = PLUGIN_COMMENT_SIZE_MSG;
 
 	if (PKWK_READONLY) return ''; // Show nothing
 
 	if (! isset($numbers[$vars['page']])) $numbers[$vars['page']] = 0;
 	$comment_no = $numbers[$vars['page']]++;
+	$commentnums++;
 
 	$options = func_num_args() ? func_get_args() : array();
 	if (in_array('noname', $options)) {
-		$nametags = '<label for="_p_comment_comment_' . $comment_no . '">' .
+		$nametags = '<label for="_p_comment_comment_' . $commentnums . '">' .
 			$_msg_comment . '</label>';
 	} else {
-		$nametags = '<label for="_p_comment_name_' . $comment_no . '">' .
+		$nametags = '<label for="_p_comment_name_' . $commentnums . '">' .
 			$_btn_name . '</label>' .
 			'<input type="text" name="name" id="_p_comment_name_' .
-			$comment_no .  '" size="' . PLUGIN_COMMENT_SIZE_NAME .
+			$commentnums .  '" size="' . PLUGIN_COMMENT_SIZE_NAME .
 			'" />' . "\n";
 	}
 (中略)
   <input type="hidden" name="digest" value="$digest" />
   $nametags
-  <input type="text"   name="msg" id="_p_comment_comment_{$comment_no}" size="$comment_cols" />
+  <input type="text"   name="msg" id="_p_comment_comment_{$commentnums}" size="$comment_cols" />
   <input type="submit" name="comment" value="$_btn_comment" />

↑

改修案2†

そもそもlabel要素が不要ではないか。
label要素は、input要素の type="radio" や type="checkbox" 等でユーザビリティの向上を図る目的で使われるのだが、type="text" ではそれほどユーザビリティの向上には貢献しない。
そこで、label要素を廃止して、id 自体を不要にしてしまうことで問題の解決を図る。

comment.inc.php

function plugin_comment_convert()
 {
 	global $vars, $digest, $_btn_comment, $_btn_name, $_msg_comment;
 	static $numbers = array();
 	static $comment_cols = PLUGIN_COMMENT_SIZE_MSG;
 
 	if (PKWK_READONLY) return ''; // Show nothing
 
 	if (! isset($numbers[$vars['page']])) $numbers[$vars['page']] = 0;
 	$comment_no = $numbers[$vars['page']]++;
 
 	$options = func_num_args() ? func_get_args() : array();
 	if (in_array('noname', $options)) {
-		$nametags = '<label for="_p_comment_comment_' . $comment_no . '">' .
-			$_msg_comment . '</label>';
+		$nametags = $_msg_comment;
 	} else {
-		$nametags = '<label for="_p_comment_name_' . $comment_no . '">' .
-			$_btn_name . '</label>' .
-			'<input type="text" name="name" id="_p_comment_name_' .
-			$comment_no .  '" size="' . PLUGIN_COMMENT_SIZE_NAME .
+		$nametags = $_btn_name . '<input type="text" name="name" size="' . PLUGIN_COMMENT_SIZE_NAME .
 			'" />' . "\n";
 	}
 (中略)
   <input type="hidden" name="digest" value="$digest" />
   $nametags
-  <input type="text"   name="msg" id="_p_comment_comment_{$comment_no}" size="$comment_cols" />
+  <input type="text"   name="msg" size="$comment_cols" />
   <input type="submit" name="comment" value="$_btn_comment" />

↑

類似問題†

PukiWikiで label 要素を使用しているソースは、comment.inc.php を除いて14個ある。

lib/html.php
- edit formで使用。通常同一文書内に複数のedit formは作られないので問題ない。
plugin/article.inc.php
- comment.inc.phpと同様の問題あり
plugin/attach.inc.php
- アップロードフォームと情報表示で使用。アクションプラグインでの動作で、複数のフォームは作られないので問題ない。

以下未調査

plugin/bugtrack.inc.php
plugin/dump.inc.php
plugin/links.inc.php
plugin/lookup.inc.php
plugin/md5.inc.php
plugin/newpage.inc.php
plugin/rename.inc.php
plugin/search.inc.php
plugin/template.inc.php
plugin/tracker.inc.php
plugin/update_entities.inc.php

↑

コメント†

もしどなたか現在の各プラグインの挙動の根拠がお解かりの方いましたら、お知らせください。別の改修案を考えて見ます。 -- Makichan 2007-10-11 (木) 11:25:02
include.inc.phpでもインクルードするページ名を$vars['page']に再設定していました。
同一ページ内に複数の#includeがある場合に同様の問題が発生します。 -- Makichan 2007-10-11 (木) 12:07:26
- そのほかの他ページ参照系プラグイン(pcomment等)は$vars['page']を再設定はしていない模様。
  ページ全体を取り込む系のプラグインは$vars['page']を再設定ということなのかな。
  calendar2.inc.phpも横にその日の記事を表示する際に$vars['page']を再設定しています。 -- Makichan 2007-10-11 (木) 12:10:36
idとってしまえばよいのではないでしょうか。少なくともcommentプラグインでは使われていないようです。comment_noという項目でコメント番号を管理しているようです。(1.4.7のcomment.inc.php,v 1.36にて確認)
ちなみに、上記修正だと#includeしたページや元のページ両方に#commentがある場合等問題が出ます。ページ毎に割り振られていた番号が、シリアルに付けられてしまいますので。plugin_comment_actionでは自分のページの#commentしか数えないのでくいちがってしまうかと。 -- ぃぉぃぉ 2007-10-11 (木) 19:48:56
- l.125辺り
```
--- <input type="text"   name="msg" id="_p_comment_comment_{$comment_no}" size="$comment_cols" />
+++ <input type="text"   name="msg" size="$comment_cols" />
```
- なるほど。comment_noはそういう使われ方をしてたんですね。それだとpageを切り替えてるのも納得できます。
  ちなみにidはプログラム的には全く使われていませんが、html的には使われています。
```
<label for="_p_comment_name_0">お名前: </label><input type="text" name="name" id="_p_comment_name_0" size="15" />
```
  こういう感じで。
  label要素を使わなければ id は不要になります。
```
お名前: <input type="text" name="name" size="15" />
```
  こうするのが妥当でしょうかね。現にpcommentなんかはlabel要素使っていませんし -- Makichan 2007-10-11 (木) 20:28:05
- いやまてよ。そもそも$comment_idとhtmlタグ中のidが同一である必要は無いのでは,,, -- Makichan 2007-10-11 (木) 20:39:23
というわけで、改修案を修正しました。疑問点としてあげていた項目はほぼ理解できたので削除しました。 -- Makichan 2007-10-11 (木) 20:48:36
なるほど、labelでidを参照していたのですか。labelのforから(文書内で一意である)idを呼ぶ、と。
自分ならidを"_p_comment_comment_{$s_page}_{$comment_no}"とかするかなぁ。やっぱりそれよりもlabelをとっちゃいそう^^;
XHTMLの仕様とか調べたことなかったので今回調べてみて勉強になりました^^ -- ぃぉぃぉ 2007-10-11 (木) 21:08:22
ぃぉぃぉさんのlabel不要というのも尤もなので、案2としてlabelをとってしまうのも書いてみました。上でも書いたけど、同様のプラグインであるpcommentなんかはlabel使ってないんで、これが妥当かもしれないですね。 -- Makichan 2007-10-11 (木) 21:37:23

お疲れ様です。突然詳細なレポートが生まれているのには驚きました (^^; 改善案1の方について: ぃぉぃぉさんが指摘した「そもそも更新が意図しない結果にならないか」という点が解消されていないように見えますが本当でしょうか。改善案2の方について: 「それほどユーザビリティに貢献しない」という主張の根拠が書かれていないです。ラベルをクリックしたときに、対応するtextフィールドにカーソルが移動する(Firefoxの場合はさらに入力候補が出る)というのがこのラベルの最低限の利点かと理解しておったのですが、そうした機構も踏まえて不要論が上がっているのかどうかが不明ですがどうなのでしょうか。 -- henoheno 2007-10-15 (月) 00:59:03
- 改善案1の件。横から失礼しますが、自分が指摘した点は解消されています。コメント欄決定に使用している変数は既存のまま残してあり、id生成用に別途変数を追加してありますので。 -- ぃぉぃぉ 2007-10-15 (月) 12:25:51
- お疲れ様です。案1については、ぃぉぃぉさんの説明のとおり、「現状の機能はそのまま維持する&問題解決のために変数を新設した」ので、多分正攻法の改修案ではないかと思います。 -- Makichan 2007-10-16 (火) 21:05:45
- 案2の根拠は、
  1. ほぼ同等の機能(利用者視点の場合)であるpcommentプラグインではlabel要素がない
  2. にもかかわらず、「pcomment」が(この点において)使いにくいという指摘がない
  という点を挙げさせていただきます。 -- Makichan 2007-10-16 (火) 21:10:22
- と、ここまで書いて何ですが、ここで「やっぱりlabelはあった方が便利だね」って話になると、じゃあほかのプラグインも積極的に使うようにしないと…という議論に発展しそうな予感(笑) -- Makichan 2007-10-16 (火) 21:17:44
改善案2 にツッコミ。「お名前:」に相当する部分が入っている変数、$_msg_comment と$_btn_name が修正後には消えています。勝手に書き足しましたけど、問題ないですよね。 -- 2007-10-16 (火) 22:26:58
- あ、ほんとだ・・・　見逃してました。ありがとうございました。 -- Makichan 2007-10-16 (火) 23:29:28
- 「お名前:」という部分をクリックした時の(昨今のWebブラウザの)挙動については同意いただいていると思ってよろしいでしょうか。 -- henoheno 2007-10-17 (水) 22:02:52

実はすでに話題になっていた…。（Ratbetaさんのコメントを参照: BugTrack2/15#raf5c0c5） -- 2007-10-17 (水) 00:34:57
- お、こっちも見逃してた・・・
  ってことは、pcomment 等にもlabel 付ける修正した方がいいですかね。 -- Makichan 2007-10-17 (水) 10:28:18
- はい。「pcommentなどのプラグインに無い」といった論点については、そもそもの評価基準(BugTrack2/15)を確認し、付ける事を考えたり、そのように取り組んだ形跡がないかどうか確認する、というのが過去を踏まえた進路として適当ではないかと思います。 -- henoheno 2007-10-17 (水) 22:01:35
ということで、こちらを却下としてBugTrack2/15に合流したほうがいいでしょうか。その方がlabel要素のあり方について包括的に議論できそう... -- Makichan 2007-10-17 (水) 10:39:33
- 話題としては既存であった様ですね。ただしこのcalendar関係の話題は、labelではなくidに関する一般的な話題のように見えていますが正しいでしょうか? だとすると生み出される答えはlabelに限らない汎用的な考え方になるような気がしています。そうした場合、逆に BugTrack2/15#raf5c0c5 の話題をこちらに吸い取って下さい。完成したそれぞれの話題がより美しく、理解するコストが少なくなる形にまとまるかもしれません。情報の構造の最終形をイメージいただいて、やってみて下さい。一回一回のアクションは、その時の最小のコストで構わないです。 -- henoheno 2007-10-17 (水) 22:07:19
- 了解です。
  label自体のあり方はBugTrack2/15に任せて、こちらではidの振り方の問題についての提案とします。 -- Makichan 2007-10-19 (金) 07:58:57
- そうなると、改修案2は不要になるかな。 -- Makichan 2007-10-19 (金) 07:59:35

total:
today:
yesterday:
now: 1