ログインボタンの二重クリック防止を実装#4481
Conversation
| $(this).attr('disabled', 'disabled'); | ||
| }); | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
テンプレートに直書きするという方法自体を見直したほうが良いのでは無いでしょうか？
確かに、テンプレートに直書きjsは他にもありますが、ある程度意味があってそう実装していたり、そもそも数も少ないです。
ビルド方法について分からないのであれば、それはPRを出す前に聞くほうがいいと思います....
(私も聞くことを忘れるケースが多いですが...)
こちらでビルド方法が記載されています。
https://baserproject.github.io/5/core_theme/javascript
There was a problem hiding this comment.
js分離のメリット・デメリット以下など分かりやすかったです。
https://qiita.com/kimascript/items/3e8d2457dd88e315bbea
There was a problem hiding this comment.
@ryuring
こちらのユーザー一覧ページのパス/baser/admin/baser-core/users/indexに合わせて、plugins/bc-admin-third/src/js/admin/usersにindex.jsを作成してこちらに二重クリック防止の内容を記述し、そのbundleファイルをplugins/bc-admin-third/templates/Admin/Users/index.phpで読み込むという形でよろしいでしょうか？
| $(this).attr('disabled', 'disabled'); | ||
| }); | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
@otsuka-star
そもそも、この対応って必要なんでしょうか？
別タブで代理ログインを開いたりしないと発生しない気がしています。
仮に別タブで開いとしてもバックエンド側のバリデーションで止まると思うのですが、そういう要望があったということでしょうか？
There was a problem hiding this comment.
おっしゃる通り、高速ダブルクリック時にログアウトするので改修して欲しいとご要望をいただいております。
ダブルクリックによるログアウトまでの流れ：
- 代理ログインは、UsersService.phpのloginToAgent()内で「logout→login」の順で処理される
- logout時に旧セッションを即座に破棄
- 高速ダブルクリックで、破棄した同IDの新規の空のセッションが読み込まれ（ログイン情報がない状態）、BaserCorePlugin.phpのunauthenticatedRedirect設定に従ってログイン画面へリダイレクトされてしまう
There was a problem hiding this comment.
@otsuka-star
ありがとうございます。理解できました！
通常の通信環境では再現しませんでしたが、Chrome DevTools の「Network」で次の設定を行い、通信速度を低下させると再現しました。
Preserve logをONThrottlingをSlow 3G
コメントいただいたとおり、根本原因はバックエンド側の代理ログイン処理にあるため、loginToAgent() を修正すべき内容だと考えます。
JavaScriptによる二重クリック防止は比較的簡単に実装できますが、同一画面での連続クリックを抑止する応急処置的な内容だと考えています。
懸念点としては以下です。
- JavaScriptが無効、未読み込み、またはエラーで動作しない環境では、二重クリックを防止できません。
(今時そんなケースないと思いますが...) - 別タブからの同時操作や、代理ログインURLへの直接アクセスは防止できません。
- バックエンド側のセッション切り替え処理に問題があるため、別の操作経路でも同様の問題が発生する可能性があります。
- JavaScriptが動作しない場合でも画面の見た目が大きく崩れないため、利用者が異常に気づきにくく、同様の問い合わせが再発する可能性があり、また要望に上がる可能性がある。
以上から、バックエンド側で競合を解消することがベストだと考えます。
ただ、バックエンドのログイン周りの処理は慎重に対応する必要があり現状では、応急処置的な対応で問題ないと認識しています。
ただし、このjs対応は一般的な実装ではなく、背景を知らない開発者には不要な処理と判断され、
後々、削除される可能性があります。(当初、私も同じように判断した。)
そのため、再発防止の観点から、発生条件・処理の目的・回避策であることをコードコメントに明記すべきだと考えます。
インラインでのスクリプトの対応について判断が分かれるところでありますが、そこさえ元のベテランのレビュー者がクリアすれば、現状問題ないと理解しました！
ゆくゆくは根本原因を修正するのが良いと思うので、後で見れるようにメモとして残します。
#4481 (comment)
バックエンドを改善する場合のメモ前提Chrome DevTools の「Network」で次の設定を行うことでダブルクリック時の不具合の再現をローカルで確認することが可能
概要通常のログイン認証には CakePHP Authentication が使われているが、 従来は代理ログイン時に フロント側の二重クリック防止JSは発生頻度を下げる効果はありますが、通信遅延やJS無効化などを考えると根本対策にはならないため、 対応として、代理ログイン開始・終了を CakePHP Authentication 標準のなりすまし機能にするのが根本的な対策として有効。 ドキュメント例： ただし根本的な対策をするのは、ログイン認証周りの修正をしないといけないため慎重に対応する必要がある。 この修正をした場合はjsでの対応は削除してメンテナンス性を上げるのが無難。 |
|
@teratai3 アドバイスありがとうございます。 |
ryuring
assigned
otsuka-star and unassigned
kaburk,
katokaisya,
otsuka-star and
ryuring
5 participants
@ryuring
ユーザー一覧ページのログインボタンの二重クリック防止を実装しましたので、ご確認宜しくお願いいたします。