Accessibility/Application passwords: Keyboard navigation issue on user profile screen

[audrasjb]

Usually, when navigating inside a form using keyboard, the enter key submits the form.

Since 5.6 which implemented the Application password section in the profile screen, using enter focuses the Application password input instead of submitting the form.

[audrasjb]

Screenshot

Trac ticket: https://core.trac.wordpress.org/ticket/52849

[alexstine]

I just tested the patch and it seems that it is still not working great. Here's what happened while testing in Firefox.

1. Focus in a profile field, First Name.
2. Type some text.
3. Press Enter.
4. Focus is moved to Application Passwords name field.
5. Profile changes saved.
6. Focus is placed back at the Application Passwords form.

We need to figure out how to disable the focus bounce to the Application Passwords section when form submit event occurs on main profile form.

It's getting better, but not quite there yet.

Also the patch needs refresh, there is a merge conflict.

Thanks.

[promz]

@alexstine I have updated the code again. I think it's working much better now.

I have replaced all submit buttons with regular <button> except the 'Update Profile' button.

Because when enter key is pressed on any input element, browser would trigger click event on all the submit buttons. This was creating unexpected behaviour as all the button click events
were fired.

[alexstine]

@promz Thanks for your reply.

After testing your latest changes, I can confirm all is working as expected. When in the main profile form, pressing Enter is just as good as selecting Update Profile. While in the Application Passwords input field, pressing Enter is just as good as clicking the Add Application Password button.

It is working fine from a screen reader perspective and I would say this greatly improves the overall UX.

The one thing left to do would be to fix the merge conflicts between your branch and upstream. Please see this guide: ​https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/resolving-a-merge-conflict-using-the-command-line

Thanks.

[promz]

Hello @alexstine,

Thanks for reviewing.

This is the part where I'm most confused. I only changed these 3 files:

src/js/_enqueues/admin/application-passwords.js
src/wp-admin/includes/class-wp-application-passwords-list-table.php
src/wp-admin/user-edit.php

The below files didn't touch but they are still automatically updated. Wondering if I should keep those changes or remove them:

src/wp-includes/assets/script-loader-packages.php
package-lock.json

Any help is appreciated 🙏

[alexstine]

Hello @promz,

I would just trash those changes, checkout with upstream branch.

Thanks.

[promz]

Hi @alexstine,

I think it's fine now. Please can you check again?

[alexstine]

@promz I think it looks good. Let me flag this ticket for more testing and hopefully we'll get it included in 5.8.

Thanks.

[audrasjb]

I left a comment in the PR to replace one esc_attr with esc_html.

Otherwise, I tested the pull request and it works as expected. Thanks!

@audrasjb I replaced it with esc_html__ now. I hope it's fine. Please feel free to review again.

[promz]

@audrasjb Thank you for the review. Good catch. I have updated the PR again. Please feel free to test again.

[audrasjb]

Dominik also made a point in his comment: Core translatable strings that are not going to be used in attributes don't need to be escaped at all :)

[promz]

@audrasjb okay, thanks. I will keep that in mind the next time. I have implemented the suggested changes. Please would you check again?

Hey @ocean90

Why is it that GitHub keeps showing the file as conflicted? I battle this also when I submit PRs.

src/wp-includes/assets/script-loader-packages.php

Does this happen when wp-env starts and changes the script-loader.php paths to another destination for testing?

This would obviously fall in another ticket, but wondering if there would be a pitfall to adding to .gitignore as I constantly have to keep checking this file out myself when I make PRs on wordpress-develop.

[sabernhardt]

In addition to refreshing against trunk, 52849.diff​ has two main edits:

• Numbered placeholders in the printf function so the name and id attributes share %1$s
• Consistent syntax for the "Add New Application Password" and "Revoke all application passwords" buttons (which includes the name and id attributes)

[joedolson]

In 51086:

Application Passwords: Allow enter key to submit profile form.

Fix the enter key in profile form fields moving focus to the application password input instead of submitting the profile update for. Replace the submit button type used for application passwords with button type="button" and ensure that the enter key's native behavior isn't overwritten.

props audrasjb, alexstine, promz, sabernhardt.
Fixes #52849.