#3534 closed enhancement (fixed)
Hide password in setup-config.php
Reported by: | xmarcos | Owned by: | joedolson |
---|---|---|---|
Milestone: | 6.3 | Priority: | low |
Severity: | normal | Version: | 2.1 |
Component: | Upgrade/Install | Keywords: | has-patch has-testing-info has-screenshots dev-feedback |
Focuses: | accessibility | Cc: |
Description
The password field in setup-config.php is set as type="text", so when you install WordPress, the password is visible as you type it. This is not only dangerous if you have someone around but it is also a possible risk if someone gets into that machine later, form fields are remembered by the browser most of the times.
In order to fix this we just need to set the field type as password, type="password".
Attachments (13)
Change History (114)
#1
@
18 years ago
- Milestone changed from 2.1 to 2.2
- Resolution set to wontfix
- Status changed from new to closed
#2
@
18 years ago
- Resolution wontfix deleted
- Status changed from closed to reopened
Matt, you seem to be talking about the post password option for blog posts... xmarcos is talking about entering the database password in setup-config.php (your answer might be the same, but I just sensed a Cool Hand Luke moment).
#3
@
18 years ago
- Owner changed from anonymous to ryan
- Status changed from reopened to new
As markjaquith points, I'm talking about the database password in here, there is a description above and a diff file with the small change.
#6
follow-up:
↓ 10
@
18 years ago
- Milestone 2.1 deleted
-1 for the current proposed solution.
If you use a field of type password, you should use a confirmation password field as well, because people can't see their password typos this way.
Also, because this is a enhancment and not a bugfix, I'm pushing it out of scope for 2.1.
#7
follow-up:
↓ 8
@
18 years ago
- Milestone set to 2.2
I like everything to have a milestone or be closed ;-)
#8
in reply to:
↑ 7
;
follow-up:
↓ 9
@
18 years ago
Replying to foolswisdom:
I like everything to have a milestone or be closed ;-)
What's keeping you:
http://trac.wordpress.org/query?status=new&status=assigned&status=reopened&milestone=
;-)
#9
in reply to:
↑ 8
@
18 years ago
Replying to Nazgul:
What's keeping you:
http://trac.wordpress.org/query?status=new&status=assigned&status=reopened&milestone=
;-)
Slowly but surely, I have been working through those. Many are no longer valid.
#10
in reply to:
↑ 6
;
follow-up:
↓ 15
@
18 years ago
- Milestone changed from 2.2 to 2.1
Replying to Nazgul:
-1 for the current proposed solution.
If you use a field of type password, you should use a confirmation password field as well, because people can't see their password typos this way.
Also, because this is a enhancment and not a bugfix, I'm pushing it out of scope for 2.1.
For the third time i think, this is the DATABASE PASSWORD, ok?
You won´t forgot it because you already have it, otherwise you won't get Wordpress running, trust me, no database password, no Wordpress.
Regarding the confirmation, it is NOT A USER-SET PASSWORD. Again you have the password already, if you mistype it, you get and sql connection error.
Now, I remember why I avoid posting tickets, people just don't take the time to read descriptions.
#11
@
18 years ago
For what it's worth, I agree that the field *should* be a password field and *should not* have a password confirmation field.
I install WordPress about twice a month, and I don't particularly like seeing all my previous database passwords pop up when the database password field receives focus, especially if I have a client looking over my shoulder.
For the other point, the person installing WP will know immediately whether they typed the password incorrectly or not when they go to the next installation page because WP will tell them it can't connect to the database. I usually just copy and paste anyway. And if people are using the one-click Fantastico installations--as I'm guessing many who would be prone to mistyping a DB password would be--they never see the field, so they can't mess it up with a wrong value.
#12
follow-up:
↓ 13
@
18 years ago
Perhaps the best solution would be to change it to a password field. Once that is submitted, a test connection is made. If it succeeds, the installation proceeds. If it fails then it returns to the previous form alerting the end user of the error (numerous other systems utilize something very similar).
#13
in reply to:
↑ 12
;
follow-up:
↓ 16
@
18 years ago
Replying to intoxination:
Once that is submitted, a test connection is made.
That sounds good. Setting milestone to 2.2 awaiting a patch.
#15
in reply to:
↑ 10
@
18 years ago
- Keywords dev-feedback 2nd-opinion added; password setup removed
- Milestone changed from 2.2 to 2.3
Replying to xmarcos:
Replying to Nazgul:
-1 for the current proposed solution.
If you use a field of type password, you should use a confirmation password field as well, because people can't see their password typos this way.
Also, because this is a enhancment and not a bugfix, I'm pushing it out of scope for 2.1.
For the third time i think, this is the DATABASE PASSWORD, ok?
You won´t forgot it because you already have it, otherwise you won't get Wordpress running, trust me, no database password, no Wordpress.
Regarding the confirmation, it is NOT A USER-SET PASSWORD. Again you have the password already, if you mistype it, you get and sql connection error.
Now, I remember why I avoid posting tickets, people just don't take the time to read descriptions.
Number one, being rude isn't going to get you anywhere. Whatsoever.
Secondly, Nazgul IS referencing the DATABASE password. If it's a passworded field, you can't see what you type in, and thus is more liable to typos. I am -1 for including this. It's not a defect -- it's a feature.
#16
in reply to:
↑ 13
@
18 years ago
Replying to foolswisdom:
Replying to intoxination:
Once that is submitted, a test connection is made.
That sounds good. Setting milestone to 2.2 awaiting a patch.
It would probably be prudent to set up a second confirmation field, too, then.
#17
@
18 years ago
- Milestone changed from 2.3 to 2.2
- Owner changed from ryan to rob1n
- Status changed from new to assigned
#18
@
18 years ago
Just an observation that two MySQL database creation processes, GoDaddy and cPanel, use different methods. GoDaddy hides the password and requires you to type the password twice. cPanel displays the password and only requires the password once.
#19
@
18 years ago
- Keywords dev-feedback 2nd-opinion removed
- Milestone 2.2 deleted
- Resolution set to wontfix
- Status changed from assigned to closed
I suppose it's up to personal choice. I think we should keep it the way it is right now, as having it in cleartext DOES limit typos (and eliminates, if the user double checks). It's not as though you enter this password in every day, like the user password.
If one of the developers want to change this, please reopen and assign to me, as I will gladly cook up a patch.
#21
@
12 years ago
It's been years since this was discussed, and I think it warrants discussion. There's something just wrong about legible passwords in text inputs. Make them type it twice.
#22
@
12 years ago
-1 on changing it. Keeping it as a text field seems like better UX to me. For similar types of interfaces, see the wifi password input interfaces in most OSes, which have a show password checkbox to ensure that you don't mistype it.
#24
@
12 years ago
It's interesting that some modern services are starting to go the other direction:
This ticket was mentioned in IRC in #wordpress-dev by tonythomas. View the logs.
11 years ago
@
11 years ago
Changed the input text values to placeholders, so they look tidy. changed password type to password. Added a show password link. JQuery is imported from http-must be fixed
#28
follow-up:
↓ 29
@
10 years ago
Let me give you a user's viewpoint: when I installed WP couple hours ago and saw password field to be plaintext, I didn't think for a second it's a feature, just a horrible typo someone made. Now I wonder if at some other place a whole DB access is made as transparent as a password was here. Passwords are expected to be entered into password fields. That is standard behaviour. If you insist on having it plaintext, at least put a memo nearby to explain it to user.
You can verify password with query or put a checkbox to make text visible, what gives user an option to opt-in for a non-standard behaviour
#29
in reply to:
↑ 28
@
10 years ago
Replying to amansurov:
You can verify password with query or put a checkbox to make text visible, what gives user an option to opt-in for a non-standard behaviour
+1 for the checkbox to show/hide the password.
#30
@
10 years ago
- Milestone set to Awaiting Review
- Resolution wontfix deleted
- Status changed from closed to reopened
Related: There has been some discussion over in #24633 about using an "eye" icon or similar concept for revealing passsword fields in the user-edit/profile screen. Maybe something to consider in 4.1 or later.
#31
@
9 years ago
- Milestone changed from Awaiting Review to 4.3
- Resolution set to fixed
- Status changed from reopened to closed
#32
@
9 years ago
- Milestone changed from 4.3 to Awaiting Review
- Resolution fixed deleted
- Status changed from closed to reopened
This is about the MySQL password.
#33
@
9 years ago
+1 for obfuscating input unless an "eye" icon or similar is checked.
Just had a very embarrassing moment installing WordPress while screen sharing with a friend. Muscle memory ensured my password was entirely outputted in plaintext before I even realized what was happening.
An "eye" icon with default obfuscation should make everyone happy.
#34
@
8 years ago
- Keywords has-patch added
- Resolution set to invalid
- Status changed from reopened to closed
I just added password_show_hide.diff which hopefully makes everyone happy.
Since the majority of the debate on this issue took place ~9 years ago, many users have argued for a clean show/hide option for displaying the password.
This patch includes internationalization for the aria labels and outputted button text.
I also removed the default value from the password and used a placeholder instead, to avoid obfuscation of the initial "password" value.
#36
@
8 years ago
Whoops. My original diff did not include the new setup-config.js file. New patch added.
#38
@
4 years ago
- Component changed from General to Upgrade/Install
- Keywords needs-refresh added
- Severity changed from minor to normal
I'd love to see this resolved - entering passwords in plain-text should be considered a security issue. Using the same interface that is used elsewhere (a password-type field with a toggle to view the entered text should the user choose to do so) makes the most sense to me. I wasn't able to get the existing patch to apply to trunk though, so marking for refresh.
This ticket was mentioned in PR #2071 on WordPress/wordpress-develop by costdev.
3 years ago
#39
- Keywords needs-refresh removed
Trac ticket: https://core.trac.wordpress.org/ticket/3534
#40
@
3 years ago
- Focuses accessibility added
- Keywords has-testing-info added; needs-unit-tests removed
- Marking as
has-testing-info
and removingneeds-unit-tests
as manual testing should suffice. - Adding
accessibility
focus to verify that there are no regressions.
Testing instructions
- Checkout PR 2071.
- Build with
npm run build
ornpm run build:dev
, depending on your preferred test environment. - Rename your
wp-config.php
file towp-config.php1
. - Navigate to
/wp-admin/
. - Click "Continue".
- Click "Let's go!".
- The "Password" field should show the "password" placeholder.
- Type a password into the "Password" field.
- The password should be masked.
- Click the "Show" button.
- The password should be revealed and the "Show" button should now read "Hide".
- Click the "Hide" button.
- The password should now be masked again.
Cleanup
- Delete the newly generated
wp-config.php
file. - Rename your
wp-config.php1
file back towp-config.php
.
#42
@
3 years ago
I noticed this ticket, and it's resolution, being set as the baseline for #9883 as well.
I'd like to make a counter-proposal, these both suggest adding a button outside the input-flow, which in the case of this ticket, even ends up on a newline under the input field, so not properly being attached to its related input area.
Is there any reason we can't use the established pattern introduced to the login screen in [46256], which has a password reveal-toggle that "overlays" the input area, so that a clear connection is kept, and the visual flow isn't broken up?
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
3 years ago
#44
@
3 years ago
We reviewed this ticekt today during the accessibiltiy team's bug-scrub.
The team suggests to first test that @costdev's patch works, and then make a decision about how to make the interfaces consistent, which is also important for accessibility.
This ticket was mentioned in Slack in #core by sabernhardt. View the logs.
3 years ago
This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.
3 years ago
This ticket was mentioned in PR #3725 on WordPress/wordpress-develop by @bgoewert.
2 years ago
#48
Trac ticket: https://core.trac.wordpress.org/ticket/3534
#49
@
2 years ago
Completed a test report using @costdev's PR 2071. Although, I wanted to also throw in that I agree with what @Clorith suggested about a consistent password reveal toggle that aligns with #9883.
I've since submitted an additional PR 3725 that combines changes from PR 2070 & PR 2071. I also completed a test report for this as well (not sure if self reports are okay). If someone else could test to verify that would be cool.
Test Report 1
Using "old" patch.
Patch tested: PR 2071
Environment
- OS: Pop_OS! 22.04
- Localhost: wordpress-develop & Docker Desktop
- WordPress: trunk
- Browser: Firefox 107, GNOME Web 43 (WebKit), Chrome 108
- Theme: N/A
- Active Plugins: N/A
Actual Results
- ✅ "Password" field shows the "password" placeholder.
- ✅️ Typing a password into the "Password" field is masked.
- ✅️ Clicking the "Show" button reveals the un-masked password.
- ✅️ "Show" button now reads "Hide".
- ✅️ Clicking the "Hide" button masks the password again.
Additional Notes
- My first test report!
- This is an old PR. I don't know if this is correct, but before testing I locally merged trunk against the PR.
Test Report 2
Using the patch I submitted.
Patch tested: PR 3725
Environment
- OS: Pop_OS! 22.04
- Localhost: wordpress-develop & Docker Desktop
- WordPress: trunk
- Browser: Firefox 107, GNOME Web 43 (WebKit), Chrome 108
- Theme: N/A
- Active Plugins: N/A
Steps to test
- (Optional) Enable a password manager browser extension.
- (Optional) The password manager icon should not overlap the password visibility toggle.
- The "Password" field should display the "password" placeholder.
- Typing a password into the "Password" field should be masked.
- Clicking the password toggle should reveal the password and change the icon from dashicon-visibilty to dashicon-hidden.
- Clicking the toggle again should mask the password and change the icon back to dashicon-visibilty.
Actual Results
- ✅️ Password manager icon does not overlap.
- ✅️ "Password" field shows a "password" placeholder.
- ✅️ The password is masked when entered.
- ✅️ Clicking the toggle un-masks the password and changes the icon.
- ✅️ Clicking the toggle again masks the password and changes the icon back.
Supplemental Artifacts
Image showing that a password manager icon does not overlap the visibility toggle and that the password is properly masked.
@ironprogrammer commented on PR #3725:
2 years ago
#50
@bgoewert, I like the direction this is headed, toward a shared implementation for the toggle feature 👍🏻
Once the above feedback is integrated, I'd love to give this a go. It should allow for #2070 to be simplified as well.
@bgoewert commented on PR #3725:
2 years ago
#51
@bgoewert, I like the direction this is headed, toward a shared implementation for the toggle feature 👍🏻
Once the above feedback is integrated, I'd love to give this a go. It should allow for #2070 to be simplified as well.
@mukeshpanchal27's suggested change has been committed. Waiting to be reviewed again.
This ticket was mentioned in Slack in #core by bgoewert. View the logs.
23 months ago
#54
@
23 months ago
- Keywords has-screenshots added
Thanks @bgoewert! I think this is almost there.
Test Report
Patch tested: https://github.com/WordPress/wordpress-develop/pull/3725.diff (applied to trunk
, testing based on instructions from comment:40)
Environment
- Hardware: MacBook Pro Apple M1 Pro
- OS: macOS 12.6.1
- Browser: Safari 16.1, Google Chrome 108.0.5359.124, Mozilla Firefox 107.0.1
- Server: nginx/1.23.2
- PHP: 7.4.33
- WordPress: 6.2-alpha-54642-src
- Password Manager: 1Password 7 v7.9.8
Actual Results
- ✅ Password field shows "password" placeholder, defaults to
input type="password"
, and "open eye" icon is displayed. - ✅ "Eye" icon is not covered by 1Password 7 suggestion toggle icon in either the hidden or visible state.
- ✅ Toggle functions as expected, hiding or showing the password.
- ⚠️ In Chrome and Firefox on macOS, the toggle button's
:focus
outline is activated when clicked, which is visually different from Safari. See Figure 4. - ⚠️ In RTL languages, the "eye" toggle shifts to the left of the field, separate from the 1Password toggle (compare this to RTL results in https://core.trac.wordpress.org/ticket/9883#comment:44 where the toggles remain grouped). See Figure 5.
Supplemental Artifacts
Figure 1: Default Password field state.
Figure 2: Field focused with password text entered.
Figure 3: Field focused with password visibility toggled on (Safari).
Figure 4: Toggle's :focus
is activated when clicked (macOS Chrome and Firefox).
Figure 5: RTL toggle placement separated from password app icon.
#55
@
23 months ago
Thanks for testing this on macOS @ironprogrammer!
It took me some time to figure out how RTL was working in WP and to research your ⚠️ results.
⚠️ In Chrome and Firefox on macOS, the toggle button's :focus outline is activated when clicked, which is visually different from Safari. See Figure 4.
Safari does this on purpose. See https://bugs.webkit.org/show_bug.cgi?id=22261
If you try this on other buttons around WP, it currently behaves in the same manner. Note though I only tested the login page.
Login page on Safari 16: https://imgur.com/eL35rzR.mp4
Compared to Chrome 108 : https://imgur.com/9npcBLU.mp4
Do we remove the visibility button border for all other browsers so that it visually aligns with Safari?
⚠️ In RTL languages, the "eye" toggle shifts to the left of the field, separate from the 1Password toggle (compare this to RTL results in https://core.trac.wordpress.org/ticket/9883#comment:44 where the toggles remain grouped). See Figure 5.
This is also currently default behavior around WP. Note this image of the login page, the password toggle is on the left and password manager on the right.
I've tried to test this by changing the primary language on Chrome. And it did change the language for the extension, but not the position of the icons (image proof). Testing on an OS with a RTL language, rather than simply changing the WP language, may change the behavior of the browser as well as password manager extensions and yield a different result (but not likely).
Is the plan to change all the other password visibility toggles to align to the right so these icons are grouped?
#56
@
23 months ago
Is the plan to change all the other password visibility toggles to align to the right so these icons are grouped?
I guess this is a yes per these: #9883:39, #9883:32, #54915
I'll push a fix for the visibility button grouping when I can.
Just need some feedback on this:
Do we remove the visibility button border for all other browsers so that it visually aligns with Safari?
#57
@
23 months ago
The RTL issue mentioned in #9883:39, #9883:32 and #54915 is about the input field, not the toggle button. An earlier version of that PR had removed the ltr
class, which correctly sets the password input to LTR direction in all languages. The setup-config
password input already sets the #pwd
direction properly, too.
The focus outline/border is necessary on the toggle button itself to show when the element is focused. If Safari does not allow that outline (and/or access to the button via keyboard), that's a serious bug in the browser that they hopefully will fix.
I'll add more comments on the PR.
@bgoewert commented on PR #3725:
23 months ago
#58
Thank you @sabernhardt for the thorough review. I tried to address all your comments. Another change I added towards a standardized password-toggle.js
is this fix for performing the toggle to the correct parent input.
However, while the icon-only button is consistent with the Login page, it is not consistent with other visibility toggle buttons in .form-table elements at larger screen sizes (on Profile and Installation pages). Andrea acknowledged that icon-only controls are an accessibility anti-pattern before committing r46256 as a "self-contained" improvement that ideally would have a different design later.
I noticed the profile page has an icon and text. If it helps with accessibility I can change the icon to text to maintain consistency with the other toggles. Would it be preferable to still keep the button as "part of" the password input? Or do we go back separating the button from the input? Do we then also change the login page to follow suit?
Also, the icon-only button on setup-config has the overlap with some password managers, as seen on the Login page (ticket:48222).
Separating the button from the input would address this.
This ticket was mentioned in PR #3827 on WordPress/wordpress-develop by @bgoewert.
23 months ago
#59
https://core.trac.wordpress.org/ticket/3534
This a text variation of #3725 to address accessibility concerns and add support for various password manager implementations. The button is separated from the input while still keeping it inline — allowing for password managers which add icons directly to the input, in contrast to those that insert another element to add their respective icon. I did not include the icon, which is inconsistent with the other toggle buttons, so as to provide maximum input space on the setup-config.php form. But the icon can easily be added back to remain consistent.
_Figure 1: Separated text password toggle w/ LastPass extension - inactive_
_Figure 2: Separated text password toggle w/ LastPass extension - active_
As a consequence of adding display: flex
to .wp-pwd
, the layout for install.php and user-new.php was also affected. I've also added changes to address this. For user-edit.php, user-profile.js#L232 applies display: block
directly to .wp-pwd
and was not affected.
_Figure 3: install.php before._
#60
@
23 months ago
- Keywords dev-feedback added
Also meant to note on pr:3827 that password-toggle.js
does not add/remove the dashicon. This is because the icon was left out of this text variation for setup-config.php
. At an attempt to keep the button inline, my initial concern was that the icon would take space away from the input.
As I noted already though, the icon can easily be re-added to remain consistent with the other password toggles. However, if password-toggle.js
is to be used commonly, either the icons would need to be removed from the other areas or we re-add the icon here and the password input is consequently smaller.
Awaiting feedback for which direction to go.
Edit: This is what it might appear like if we re-add the icon and kept it inline.
This ticket was mentioned in Slack in #accessibility by bgoewert. View the logs.
22 months ago
#62
@
22 months ago
Added two new changes to pr:3827. One that re-adds the icon to the toggle button to remain consistent with the other existing password toggles — except for the login page which is currently icon-only. And another change that slightly increases the input size on install.css
to allow for the bigger button as well as fix a few orphans in the descriptions on both setup-config.php
and install.php
.
Figure 1: Original setup-config.php with a relatively small password input and orphaned words on the last two descriptions.
Figure 2: New setup-config.php after input size adjustment.
Figure 3: Original install.php with an orphaned word on the password input description.
This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.
22 months ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
22 months ago
#65
@
22 months ago
Tried to look into the RTL note on comment:54 some more because I am unfamiliar with such things. From what I can gather (Apple, Material Design), controls like this should be mirrored. The password manager extensions may have their own reasons or limitations for not doing this. It may be a non-issue now though since the toggle, at least on pr:3827, is now a traditional button and not an inline icon.
#66
@
22 months ago
Test Report
Patch tested: PR:3827
Environment
- OS: Pop!_OS 22.04
- Web Server: Docker-Desktop & wordpress-develop
- PHP: 7.4.33
- WordPress: 6.2-alpha-54642-src
- Browser: Firefox 108, Chrome 109
- Theme: Twenty Twenty-Three
- Active Plugins: None
- OS: macOS Monterey (via LambdaTest)
- Browser: Safari 16
Actual Results
- ✅ Password field shows "password" placeholder.
- ✅ Field is masked by default.
- ✅ Toggle functions as expected, hiding or showing the password.
- ✅ Works with password manager extension icons inserted via an element or background image.
- ✅ Button is mirrored for RTL languages.
- ✅ ARIA label changes on toggle.
- ❌ Input is not full width on narrower screens. See figure 6.
Supplemental Artifacts
Figure 1: Default state with "password" placeholder and Keeper Security icon. (Chrome)
Figure 2: Password field focused with LastPass icon. (Chrome)
Figure 3: Password visibility toggled on and focused with LassPass icon. (Chrome)
Figure 4: Password visibility toggled on. (Safari)
Figure 5: RTL language with toggle button focused.
Figure 6: Narrower screen (768px) with toggle button focused.
#67
@
22 months ago
Updated PR:3827 to fix the input not being full width on narrow screens which was noted in comment:66. Also reverted the custom width/padding that was being applied to .pwd-toggle
so it defaults to the regular button width/padding.
Figure 1: Password input on narrow screen with toggle button enabled and focused.
This ticket was mentioned in Slack in #core-upgrade-install by bgoewert. View the logs.
22 months ago
@audrasjb commented on PR #3827:
22 months ago
#69
Hello, there is a small conflict in steup-config.php since I committed [55110]
@bgoewert commented on PR #3827:
22 months ago
#70
Thanks @audrasjb. I just merged trunk to address the conflict.
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
22 months ago
#72
@
22 months ago
Thanks @xmarcos for the ticket.
This ticket was discussed in the recent bug scrub.
Can someone from the testing team (@adeltahri or @robinwpdeveloper) take a look and share the feedback?
Props to @costdev
#73
follow-up:
↓ 75
@
22 months ago
Test Report
Patch tested: 3827
Environment
- OS: Mac Mini 2020 M1
- Web Server: Docker-Desktop & wordpress-develop
- PHP: 7.4.33
- WordPress: 6.2-alpha-54642-src
- Browser: Arc, Firefox 108, Chrome 109, Safari 16
- Theme: Twenty Twenty-Three
- Active Plugins: None
Actual Results
✅ Password field shows "password" placeholder.
✅ Field is masked by default.
✅ Toggle functions as expected, hiding or showing the password.
✅ Works with password manager extension icons inserted via an element or background image.
✅ Button is mirrored for RTL languages.
✅ ARIA label changes on the toggle.
❌ Button show/hide on setup-config.php?step=1 not showing.
❌ Button show/hide on wp-login.php not inside the input, password filed not 100% width.
Supplemental Artifacts
Figure 1:
https://d.pr/i/WAwW5Z
Figure 2:
https://d.pr/SL00Tv
This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.
22 months ago
#75
in reply to:
↑ 73
@
22 months ago
Replying to adeltahri:
❌ Button show/hide on setup-config.php?step=1 not showing.
This may be the hide-if-no-js
class. I was unable to reproduce without blocking JavaScript. I'm surprised that toggling worked if the button was not displayed.
Can you verify what browser this was in, if not all of them? Maybe my testing environment is not correct? Although, I tested with npm run build:dev
as well as npm run build
and wp-env
mapped to ./build
. both of which display the button.
Maybe someone else can give thoughts on this?
❌ Button show/hide on wp-login.php not inside the input, password filed not 100% width.
I failed to test wp-login.php
on comment:66. Thank you for catching this issue. I know what is causing this and I'll get it fixed.
#76
@
22 months ago
Updated PR 3827 to address the login toggle button and other small things pointed out on the PR. Still unable to reproduce the button not showing at all without disabling JavaScript.
If someone, @adeltahri or @robinwpdeveloper, could test again.
#77
@
22 months ago
Hello @bgoewert, I'll re-test it and npm run build:dev
and back with the full report.
#78
@
22 months ago
Test Report
Patch tested: PR:3827
Environment
- OS: Mac Mini 2020 M1
- Web Server: Docker-Desktop & wordpress-develop
- PHP: 7.4.33
- WordPress: 6.2-alpha-54642-src
- Browser: Arc, Firefox 108, Chrome 109, Safari 16
- Theme: Twenty Twenty-Three
- Active Plugins: None
Actual Results
✅ Button show/hide on setup-config.php?step=1 (GIF 1).
✅ Works with password manager extension Lastpass (GIF 2).
✅ Button show/hide on wp-login.php (GIF 2).
✅ Tested on install.php (GIF 3).
Supplemental Artifacts
I have tested with npm run build:dev
and found no issues with the patch. So I think we are good to go.
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
22 months ago
This ticket was mentioned in Slack in #core-upgrade-install by pbiron. View the logs.
18 months ago
This ticket was mentioned in Slack in #core by chaion07. View the logs.
18 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
17 months ago
This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.
17 months ago
This ticket was mentioned in PR #4677 on WordPress/wordpress-develop by @joedolson.
17 months ago
#88
Show/hide password toggle for config; updates wrappers;
Trac ticket: https://core.trac.wordpress.org/ticket/3534
#89
@
17 months ago
- Keywords changes-requested removed
@sabernhardt I've added a new PR with the requested changes; if you can give it a look over.
@
17 months ago
Install: the Password field needs to be full width on smaller screens, and it has extra padding on the right side in RTL
#90
@
17 months ago
- On the Install page, the Password field and button do not go to the opposite edge to match the other fields. Using 100% might help a little at larger screen widths, and it needs to be that wide on small screens.
.password-input-wrapper { display: inline-block; width: 100%; }
- In RTL on smaller screens,
setup-config
positions the button above the input with no left padding..form-table td input[type="password"] { /* rtl:ignore */ padding-right: 2.5rem; }
could be changed to#pwd { padding-right: 2.5rem; }
(The button is on the left on larger screens, so it should stay on the same side for smaller screens.)
- Consider giving the middle cells for
setup-config
a fixed width of about 300 pixels, and then reset it toauto
(or 100%) for smaller screens. I don't like how the inputs expand and the description column shifts when clicking the toggle button. The Password field can also be quite narrow in languages with wide translations of Show and/or Hide..setup-config-fields th + td { width: 300px; } @media screen and (max-width: 782px) { .setup-config-fields th + td { width: 100%; } }
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
17 months ago
#92
@
17 months ago
Making .password-input-wrapper
100% width at desktop sizes breaks the layout of these forms in user-new and user-edit, so not a great option. Went with making an exception when in .form-table.
Also noticed that the structure of these controls was updated in user-new.php
for this patch, but not in user-edit.php
, so made those updates as well.
Switched the setup config page to two columns. This matches the layout used on install.php, so I think this actually makes a more consistent interface through the install and setup process.
Noticed while I was in here that the description fields on install.php aren't explicitly associated with their respective inputs, so I'll be opening a new issue to fix that issue shortly.
#94
@
17 months ago
I'm wondering whether we should shift all instances of the password toggle to use the new dedicated file, as well. If we're introducing this file, we should really remove the duplicate behaviors.
Although perhaps that should be a follow-up.
#95
@
17 months ago
@sabernhardt I'm not going to map this script into profile pages in this ticket; it makes more sense for you to do it in #57157 post commit.
#96
@
17 months ago
- Keywords commit added
I'm satisfied. If additional issues crop up, they can be fixed during beta. Marking for commit.
@joedolson commented on PR #4677:
17 months ago
#98
Closed in r56008
#99
@
17 months ago
- Keywords commit removed
The visible text of "Show" and "Hide" from the script is not translated with WP6.3 beta2. I thought this might be fixed automatically closer to the release, but I did not have those translations for the toggle button on install.php
either (versions 5.5 to 6.3b2). That needs a new ticket to fix separately: #58696. (The login and profile button translations are fine.)
@bgoewert commented on PR #3725:
14 months ago
#100
Closed in r56008
@bgoewert commented on PR #3827:
14 months ago
#101
Closed in r56008
The reason we don't do this is because the people most likely to use this feature are the least able to view source or something to see the password they typed in and forgot.
The password field is not an access method, it's a setting. We wouldn't hide the title, or the categories, or anything else we feel needs to be accessible and editable by the author.
As for the "if you have someone around" argument, if the person is that paranoid they can always minimize that sidebar box.