Move inline JS to external script files#814
Move inline JS to external script files#814aslamdoctor wants to merge 4 commits intoWordPress:masterfrom
Conversation
Extract 6 inline script blocks from 4 PHP files into 5 new external JS files under providers/js/, improving CSP compatibility and enabling proper WordPress script dependency management. New files: - totp-admin-qrcode.js: QR code generation for TOTP setup - totp-admin.js: TOTP setup form, checkbox focus, reset key - backup-codes-admin.js: Backup code generation, copy, download - two-factor-login.js: Shared login page authcode clear/focus - two-factor-login-authcode.js: Numeric-only enforcement, auto-submit The customizer messenger one-liner uses wp_add_inline_script() instead of a raw <script> tag. Fixes WordPress#812
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
The "Reset authenticator app" button's AJAX response injects HTML with a QR code placeholder, but the QR code generator script only runs on page load and never re-triggers. Additionally, the qrcode library wasn't loaded when TOTP was already configured. Add qr-code-generator as a dependency of totp-admin script and generate the QR code in JavaScript after the reset response HTML is inserted.
Move all var declarations to the top of their respective function scopes.
masteradhoc
left a comment
masteradhoc
left a comment
There was a problem hiding this comment.
Thanks for the fast fix regarding my comment about the QR-Code not rendering after resetting the TOTP. Checked it now and looks good from my side.
|
For big migrations like this, I've found that copilot is really good at catching stuff my eyes gloss over. Just wanna make sure it doesn't surface anything significant. |
There was a problem hiding this comment.
Pull request overview
This PR extracts previously inline JavaScript from provider PHP templates into external script files under providers/js/, enabling better CSP compatibility and allowing WordPress dependency management (notably ensuring wp-api-request is properly enqueued when needed).
Changes:
- Added 5 new external JS files for login behaviors and provider admin UIs (TOTP + backup codes).
- Updated provider PHP to register/enqueue these scripts and pass dynamic values via
wp_localize_script(). - Replaced a raw customizer
<script>snippet withwp_add_inline_script().
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
providers/js/two-factor-login.js |
New login helper script (focus/clear authcode). |
providers/js/two-factor-login-authcode.js |
New login authcode input enforcement + autosubmit logic. |
providers/js/totp-admin.js |
New admin/profile TOTP setup/reset interactions via REST calls. |
providers/js/totp-admin-qrcode.js |
New TOTP QR-code rendering script using qrcode generator. |
providers/js/backup-codes-admin.js |
New backup codes UI (generate/copy/download) via REST calls. |
providers/class-two-factor-totp.php |
Registers/enqueues new TOTP scripts and localizes REST params. |
providers/class-two-factor-email.php |
Enqueues shared login script instead of inline <script>. |
providers/class-two-factor-backup-codes.php |
Registers/enqueues new backup codes script and localizes REST params. |
class-two-factor-core.php |
Registers login scripts and switches customizer snippet to wp_add_inline_script(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Also previously: |
- Add $hook_suffix parameter to backup codes enqueue_assets() for PHP 8+ - Replace trimStart() with ES5-compatible regex in login authcode - Fix submit button disable using querySelector instead of form.submit - Guard responseJSON access in TOTP admin fail handler - Fix SVG ARIA: use setAttribute with role="img" instead of DOM properties - Localize QR code aria-label string for translation - Move wp_add_inline_script before login_footer action
Summary
<script>blocks from 4 PHP files into 5 new external JS files underproviders/js/, improving CSP compatibility and enabling proper WordPress script dependency management (fixes issues like Missing script dependency: Uncaught TypeError: wp.apiRequest is not a function #558 wherewp.apiRequestwas unavailable)totp-admin-qrcode.js,totp-admin.js,backup-codes-admin.js,two-factor-login.js,two-factor-login-authcode.jswp_add_inline_script()instead of raw<script>tagnpm run lint:js(WordPress ES5 rules)Test plan
jquery,wp-api-request,qrcode.js)Fixes #812