Project

General

Profile

Bug(バグ) #1193

確認キーワードを誤入力または未入力の場合にエラーメッセージが表示されない

Added by Mutsumi Imamura over 9 years ago. Updated about 2 years ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
2010-06-22
Due date:
% Done:

100%

3.6 で発生するか:
Yes (はい)
3.8 で発生するか:
Unknown (未調査)

Description

Overview (現象)

新規登録画面で確認キーワードを誤入力または未入力の場合にエラーメッセージが表示されない

確認環境

  • OpenPNE3.5.x
  • OpenPNE3.4.x
  • 確認ブラウザ:IE8

再現方法

  1. opAuthMailAddressPluginの設定で招待なしでの登録を許可するに変更する
  2. ログイン画面より新規登録リンクをクリックする
  3. 新規登録画面で確認キーワードの答えを誤入力または未入力でsubmitする

Causes (原因)

Way to fix (修正内容)


Related issues

Related to OpenPNE 3 - Backport(バックポート) #2723: 確認キーワードを誤入力または未入力の場合にエラーメッセージが表示されない Fixed(完了) 2010-06-22
Related to OpenPNE 3 - Backport(バックポート) #2724: 確認キーワードを誤入力または未入力の場合にエラーメッセージが表示されない Fixed(完了) 2010-06-22
Related to OpenPNE 3 - Bug(バグ) #2847: #1193 の修正により op_include_form() を使用している箇所で renderHelp() の内容が出力されていない Won't fix(対応せず) 2012-02-27

Associated revisions

Revision 872e491b (diff)
Added by Minoru Takai almost 8 years ago

(fixes #1193) use renderError() and render() instead of renderRow() to output error messages in global/partsForm

History

#1 Updated by Rimpei Ogawa over 9 years ago

  • 3.6 で発生するか set to Yes

#2 Updated by Kousuke Ebihara about 9 years ago

  • Project changed from opAuthMailAddressPlugin to OpenPNE 3

#3 Updated by Kousuke Ebihara about 9 years ago

CAPTCHA はコア側で用意している機能です

#4 Updated by Yuma Sakata almost 8 years ago

  • 3.6 で発生するか changed from Yes to Yes (はい)
  • 3.4 で発生するか set to Yes (はい)

再現確認

以下バージョンで再現確認できました。

  • 3.6.1
  • 3.4.18

備考

IE8 環境で再現確認済みです。

#5 Updated by Yuya Watanabe almost 8 years ago

  • Target version set to OpenPNE 3.7.0

#6 Updated by Yuya Watanabe almost 8 years ago

調査途中

下記部分ではバリデートエラーとして例外を投げているが,この例外がうまく機能していない?

lib/form/opCaptchaForm.class.php

 39   public function validateCaptchaString($validator, $value, $arguments)
 40   {
 41     $answer = '';
 42     if (isset($_SESSION['captcha_keystring']))
 43     {
 44       $answer = $_SESSION['captcha_keystring'];
 45       unset($_SESSION['captcha_keystring']);
 46     }
 47 
 48     if ($value['captcha'] !== $answer)
 49     {
 50       $error = new sfValidatorError($validator, 'invalid');
 51 
 52       throw new sfValidatorErrorSchema($validator, array('captcha' => $error));
 53     }
 54 
 55     return $value;
 56   }

例えばopAuthMailAddressPluginでは下記のようにフォームを用いている.

plugins/opAuthMailAddressPlugin/lib/form/opRequestRegisterURLForm.class.php

 37     if (sfConfig::get('op_is_use_captcha', false))
 38     {
 39       $this->embedForm('captcha', new opCaptchaForm()); 
 40     }

#7 Updated by Minoru Takai almost 8 years ago

  • Status changed from New(新規) to Accepted(着手)
  • Assignee set to Minoru Takai

調査します。

#8 Updated by Minoru Takai almost 8 years ago

まず、 note-6 で示されている独自のコールバックのバリデータ部分を通っているか検証しました。 throw の直前で適当に echo などをして exit してみましたが、ここは通っていました。

独自のコールバックでのバリデータの記述が悪いのか、あるいは captcha ウィジェットに対する全てのエラーメッセージが渡っていないのか、あるいはエラーメッセージが渡っているがテンプレート側で出力されていないだけなのかを区別するため、とりあえず次のような変更を加えて動作を確認してみました。

diff --git a/lib/form/opCaptchaForm.class.php b/lib/form/opCaptchaForm.class.php
index c9f8ae2..ad33c0c 100644
--- a/lib/form/opCaptchaForm.class.php
+++ b/lib/form/opCaptchaForm.class.php
@@ -20,7 +20,7 @@ class opCaptchaForm extends BaseForm
   public function configure()
   {
     $this->setWidget('captcha', new opWidgetFormCaptcha());
-    $this->setValidator('captcha', new sfValidatorPass());
+    $this->setValidator('captcha', new sfValidatorString(array('max_length' => 4)));

     $formatter = new sfWidgetFormSchemaFormatterList($this->widgetSchema);
     $formatter->setRowFormat("<li>%field%%help%\n%hidden_fields%</li>\n");

これは captcha に入力した文字列の長さが 4 を超える場合はエラーとなるようなバリデータを使った状態です。これで正しい captcha を入力しても、長さに関してエラーとなるようになりますが、これを実際に試したところ、エラーメッセージは表示されませんでした。

これで、コールバックの独自バリデータが悪いというわけではなさそうであることが確認できました。

次に、エラーメッセージが渡っているかどうかを確認するため、次の変更を加えました。

diff --git a/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegisterURLInput.php b/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegi
index 4af40e8..12f91ee 100644
--- a/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegisterURLInput.php
+++ b/apps/pc_frontend/modules/opAuthMailAddress/templates/requestRegisterURLInput.php
@@ -1,7 +1,12 @@
 <?php slot('_request_register_url_body') ?>
 <?php echo __('Please input your e-mail address. Invitation for %1% will be sent to your e-mail address.', array('%1%' => $op_config['sns_name'])) ?>
 <?php end_slot(); ?>
-
+<?php
+  //var_dump($form['captcha']->hasError());
+var_dump($form['captcha']->renderError());
+//var_dump($form['captcha']);
+exit;
+?>
 <?php echo op_include_form('requestRegisterURL', $form, array(
   'title' => __('Register'),
   'body' => get_slot('_request_register_url_body'),

これを実行したところ、

string '  <div class="error"><ul class="error_list">
    <li>4文字以内で入力してください。</li>
    <li>正しくありません。</li>
  </ul></div>
' (length=157)

のような結果が得られました。つまりテンプレート側にエラーメッセージが渡っており、取得できることが確認できました。

embedForm を使っているせいか分かりませんが、フォームの入力内容に不備があって再表示される際に captcha ウィジェットに関してはエラーメッセージが自動的に表示されていない状態であったということです。

補足

http://www.symfony-project.org/more-with-symfony/1_4/ja/06-Advanced-Forms に「カスタムバリデータを作る」という節がありますが、ここで次のように示されています。

      // この埋め込みフォームでのエラー
      if (count($errorSchemaLocal))
      {
        $errorSchema->addError($errorSchemaLocal, (string) $key);
      }
    }

    // エラーをメインフォームへスロー
    if (count($errorSchema))
    {
      throw new sfValidatorErrorSchema($this, $errorSchema);
    }

embedForm を用いる場合はこのようにメインとなるフォームにエラーを渡す必要があるのかもしれません。

#9 Updated by Minoru Takai almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

更新履歴 872e491b1ecf5c9606201c91cdc724008a4dcdb6 で適用されました。

#10 Updated by Minoru Takai almost 8 years ago

note-8 から更に調べて修正しました。 note-9 で修正していますが、修正差分は次のものです。

commit 872e491b1ecf5c9606201c91cdc724008a4dcdb6
Author: mtakai <m.takai@tejimaya.com>
Date:   Mon Feb 6 23:42:50 2012 +0900

    (fixes #1193) use renderError() and render() instead of renderRow() to output error messages in global/partsForm

diff --git a/apps/pc_frontend/templates/_partsForm.php b/apps/pc_frontend/templates/_partsForm.php
index dbac137..5b826c3 100644
--- a/apps/pc_frontend/templates/_partsForm.php
+++ b/apps/pc_frontend/templates/_partsForm.php
@@ -94,7 +94,10 @@ if ($options['mark_required_field']
   $hasRequiredField = true;
 }
 ?>
-<?php echo $field->renderRow($attributes, $field->renderLabelName().$labelSuffix) ?>
+<tr>
+<th><?php echo $field->renderLabel($field->renderLabelName().$labelSuffix); ?></th>
+<td><?php echo $field->renderError(), $field->render($attributes); ?></td>
+</tr>
 <?php endforeach; ?>
 <?php endforeach; ?>
 <?php echo $options->getRaw('lastRow') ?>

生じていた問題

新規登録用URL送信フォーム(招待なしの新規登録フォーム)、および招待フォームで op_include_form() が使われており、それにより global/_partsForm.php が呼ばれていましたが、 partsForm では少なくとも CAPTCHA に関してウィジェットに対するエラーが出力されていませんでした。

この問題は CAPTCHA に限らず partsForm によって表示される全てのフォーム項目に共通する内容ではないかと思いましたが、このコメントでの修正を適用する前でも、設定変更ページのフォームなどでは、 CAPTCHA 以外の項目についてはエラーメッセージが出力されていました。

原因

symfony の form クラスが用意しているメソッドや使い方をあまり把握していませんが、 renderRow() を用いており、 CAPTCHA の場合に内部で renderError() が行なわれていませんでした。 CAPTCHA に限って出力されていなかった原因は調査できていません。

そのため、 renderRow() メソッドと同等の HTML を直に書き、 render() 相当の出力の直前に renderError() を出力するように書き換えました。

たまたま見つけましたが、 #911a79b33cc の editSuccess.php でも、目的が違うかもしれませんが同様の修正を行なっているようです。

関連する問題

このチケットの問題とは異なりますが、関連する問題を見つけたのでコメントしておきます。

  • opAuthMailAddressPlugin の新規登録用URL送信フォーム(招待なしの新規登録フォーム)、および招待フォームにおいて、メールアドレスのバリデートにおけるエラーメッセージがウィジェットに対するものではなく globalError に出力されています
  • 今回の修正にもありますが、フォームコントロールのラベル名には、必須項目である場合に <strong>*</strong> のような文字列が末尾に付きますが、バリデータが sfValidatorPass か sfValidatorSchema の場合にはこれが出力されないようになっています(新規登録用URL送信フォームや招待フォームでは、メールアドレスも CAPTCHA も必須であり $validator->getOption('required') が true であるのに、必須マークが出力されていません)
    apps/pc_frontend/templates/_partsForm.php
    88-if ($options['mark_required_field'] 
    89-  && !($validator instanceof sfValidatorPass)
    90-  && !($validator instanceof sfValidatorSchema)
    91-  && $validator->getOption('required'))
    92-{
    93-  $labelSuffix = ' <strong>*</strong>';
    94-  $hasRequiredField = true;
    95-}
    96-?>
    97-<tr>
    98-<th><?php echo $field->renderLabel($field->renderLabelName().$labelSuffix); ?></th>
    99-<td><?php echo $field->renderError(), $field->render($attributes); ?></td>
    100-</tr>
    

レビュアーおよびテスターの方へ

  • 修正内容が妥当かどうかは、レビュアーの方のチェックに委ねます(実装者としても symfony のフォーム周りや、 partsForm パーツのもともとの実装の意図を知らないので、これが適切でない修正の可能性がある)。
  • 今回、 partsForm という共通のテンプレートファイル( op_include_form() を使うテンプレート全てから参照されるファイル)を書き換えています。そのため、動作テストに関しては、このチケットで言及されているページだけでなく、他のフォームページ(例えば、設定変更ページ)の正常系動作も確認したほうが良いかもしれません。
    • 必須マークやHTMLの文法など、細かい点については実装者である私自身が注意していますが、テスターテストの目的と、その動作テストによって保証されるべき内容について意識されるべきであるということです。

#11 Updated by Kousuke Ebihara almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#12 Updated by Minoru Takai almost 8 years ago

このチケットでの修正についてコメントします。

symfony の form クラスが用意しているメソッドや使い方をあまり把握していませんが、 renderRow() を用いており、 CAPTCHA の場合に内部で renderError() が行なわれていませんでした。 CAPTCHA に限って出力されていなかった原因は調査できていません。

と note-10 で言っていますが、これは CAPTCHA フォームが embedForm で扱われていることが原因かもしれません(が、この辺りに詳しくないので断言できません)。 render() メソッドは以下のように分岐があります。

lib/vendor/symfony/lib/form/sfFormField.class.php
115-  function render($attributes = array())
116-  {
117-    if ($this->parent)
118-    {
119-      return $this->parent->getWidget()->renderField($this->name, $this->value, $attributes, $this->error);
120-    }
121-    else
122-    {
123-      return $this->widget->render($this->name, $this->value, $attributes, $this->error);
124-    }
125-  }

ところで、 note-10 で行なった修正では、

-<?php echo $field->renderRow($attributes, $field->renderLabelName().$labelSuffix) ?>
+<tr>
+<th><?php echo $field->renderLabel($field->renderLabelName().$labelSuffix); ?></th>
+<td><?php echo $field->renderError(), $field->render($attributes); ?></td>
+</tr>

のように renderRow() の代わりに「renderError(), render()」を書いていますが、 renderHelp() の内容が出力されていません。 renderHelp() が出力されない問題は別チケット #2847 で扱います。

#13 Updated by Shouta Kashiwagi almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです。

#14 Updated by Chiharu Nakajima about 2 years ago

3.6対応済み( #2723 )
3.8発生せず

Also available in: Atom PDF