Closed Bug 502258 Opened 13 years ago Closed 10 months ago

Option to Unhide Entry of Password (AKA "show password" equivalent for current mobile app/webpage idiom)

Categories

(Core :: Layout: Form Controls, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: david, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete)

Attachments

(6 files, 6 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.22) Gecko/20090605 SeaMonkey/1.1.17
Build Identifier: 

Request a preference option such that, if set, passwords are visible instead of being asterisks.  To allay security concerns, this option should require prior entry of the master password before being enabled.  If selected beforehand, the option should expose the entered password while it is being typed.  If selected afterwards, the option should expose the entered password after it was typed.   

Reproducible: Always




As I get older, my fingers do not always do what I want them to do.  When entering a password (especially for a site that blocks the use of pre-stored passwords from Password Manager), I often have to re-enter the password because I mistyped it.  Some sites, however, lock an account for a user ID if there are multiple password errors within a short period of time.  This RFE would allow me to see what I typed and thus reduce the likelihood of being locked out.  

See Jakob Nielsen's "Stop Password Masking" at the cited URI for further justification for this RFE.  Nielson is a professional consultant on Internet useability issues.  

The extension Show Password On Input at <https://addons.mozilla.org/en-US/firefox/addon/6143/> is a response to my similar bug 232050 for the master password.  Per Nielson's commentary, these should not be extensions; instead, they should be inherent in the Toolkit.
Duplicate of this bug: 545939
The Show my Password extension at <http://netcat.ath.cx/extensions.html> addresses this issue very well.  However, I strongly suggest that the capabilities of that extension be incorporated into Toolkit's Password Manager so that future changes to the Password Manager do not require corresponding changes to the extension.
Pretty sure we'd WONTFIX this but I'll leave it open for now.
Whiteboard: [WONTFIX?]
There have been 91,266 downloads of the Show My Password extension.  This indicates user interest in this capability.  Yes, an extension CURRENTLY provides the capability.  This RFE, however, would mean that the capability would remain currently available whenever Password Manager is changed.  

Therefore, I request justification for why this might be WontFix.
This duplicates bug #368318.  A WONTFIX should be explained, considering the clear need.  Would a patch be welcome?
This is a Toolkit/Password Manager bug (RFE).  Bug #368318 is against Firefox/General.  Thus, this bug report is more specific to what needs to be changed and also provides better justification.  Although bug #368318 is older, I would suggest that it be closed as a duplicate of this one.  

Note that the Show my Password extension (for passwords entered on Web pages, cited in comment #2) now requires an AMO override for compatibility with current versions of Mozilla-based products.  That would not be necessary if this RFE were implemented.
Whiteboard: [WONTFIX?]
This bug hasn't been WONTFIXed. The whiteboard is marked that way because I feel like we probably should WONTFIX it, but I wasn't sure enough to actually close it.

I still feel that way and think this is something best left to an extension at this point. The one mentioned in bug 368318 claims to be compatible through Firefox 7. (https://addons.mozilla.org/en-US/firefox/addon/unhide-passwords/)
Whiteboard: [WONTFIX?]
"I still feel that way and think this is something best left to an extension at this point."

If an extension could do it all, that could be OK.  However, the given extension
is more or less the equivalent of a greasemonkey script that changes the
input type=password to type=text.  Thus it may help some html, but does nothing
for HTTP-401 type password prompt dialog boxes or the like.
An extension is at best a temporary work-around, especially when it needs to be modified or else requires an AMO override every time I update my browser.
There has been talk about allowing this temporarily like how IE11 does with an eye icon in the password field. While pressed, the password is visible. This isn't really a password manager bug though since it's not about saved passwords.
Component: Password Manager → Layout: Form Controls
Product: Toolkit → Core
Whiteboard: [WONTFIX?] → [DUPEME]
Duplicate of this bug: 368318
Temporarily exposing passwords would be acceptable if this includes exposing while a password is being typed.  

In addition to the Show Password On Input extension for master passwords, I now have the Show my Password extension for individual passwords.  While this is somewhat helpful, the exposed password too easily becomes hidden again.  I would hope that an implementation for this bug report provides a less transient exposure.
Duplicate of this bug: 918538
Duplicate of this bug: 1111123
Summary: Option to Unhide Entry of Password → Option to Unhide Entry of Password (AKA "show password" equivalent for current mobile app/webpage idiom)
Stephen, I heard that this was brought up in Whistler and that the only thing that's blocking platform folks from fixing this bug is icons. Do we have an SVG icon that they could use for this? SVG should allow for us to invert the color of the icon as necessary. Ehsan, is that all we would need from UX?
Flags: needinfo?(shorlander)
Flags: needinfo?(ehsan)
Do we want to use the native widget where applicable? e.g. Windows 8+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Stephen, I heard that this was brought up in Whistler and that the only
> thing that's blocking platform folks from fixing this bug is icons. Do we
> have an SVG icon that they could use for this? SVG should allow for us to
> invert the color of the icon as necessary. Ehsan, is that all we would need
> from UX?

Yes, I have already asked my intern Michael to look into this.

We will need help from the UX team on both the icon, and the interaction (whether we want to do something similar to native Windows 8+, or something similar to Edge on Windows 10, or something else).

I guess an SVG icon would work.

(In reply to Matthew N. [:MattN] (behind on reviews) from comment #17)
> Do we want to use the native widget where applicable? e.g. Windows 8+

Probably.  We need to investigate to see whether Windows provides the native widget in a way that we can draw it in the widget layer using instructions from the OS, or whether we need to imitate the native widget ourselves.
Flags: needinfo?(ehsan)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Stephen, I heard that this was brought up in Whistler and that the only
> thing that's blocking platform folks from fixing this bug is icons. Do we
> have an SVG icon that they could use for this? SVG should allow for us to
> invert the color of the icon as necessary. Ehsan, is that all we would need
> from UX?

I am not entirely sure what is being proposed here or what icons would be required. Are we talking about adding a button to show passwords in all password fields? Would it be a toggle? A press and hold to reveal?

Ryan has done more thinking here than I have.
Flags: needinfo?(shorlander) → needinfo?(rfeeley)
When I submitted this bug report, I anticipated an implementation not much different from the Show my Password extension from <https://addons.mozilla.org/en-US/seamonkey/addon/8016/>.  

If no preference was set beforehand to show passwords, the pull-down context menu should have a menu item for this.  Furthermore, the feature should not work until after the master password has been entered (unless, of course, there is no master password, in which case the feature would work without it).
(In reply to Stephen Horlander [:shorlander] from comment #19)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> > Stephen, I heard that this was brought up in Whistler and that the only
> > thing that's blocking platform folks from fixing this bug is icons. Do we
> > have an SVG icon that they could use for this? SVG should allow for us to
> > invert the color of the icon as necessary. Ehsan, is that all we would need
> > from UX?
> 
> I am not entirely sure what is being proposed here or what icons would be
> required. Are we talking about adding a button to show passwords in all
> password fields? Would it be a toggle? A press and hold to reveal?
> 
> Ryan has done more thinking here than I have.

Yeah, I think it'd be pretty safe to go with a press-and-hold, similar to what Edge does.

For default-styled password fields, Edge shows a grey background on hover, and a black background on mousedown, inverting the icon stroke. Password fields that have a custom background-color and text-color show the icon stroke in the same color as the text color and don't invert the stroke of the icon on mousedown.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #21)
> (In reply to Stephen Horlander [:shorlander] from comment #19)
> > (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> > > Stephen, I heard that this was brought up in Whistler and that the only
> > > thing that's blocking platform folks from fixing this bug is icons. Do we
> > > have an SVG icon that they could use for this? SVG should allow for us to
> > > invert the color of the icon as necessary. Ehsan, is that all we would need
> > > from UX?
> > 
> > I am not entirely sure what is being proposed here or what icons would be
> > required. Are we talking about adding a button to show passwords in all
> > password fields? Would it be a toggle? A press and hold to reveal?
> > 
> > Ryan has done more thinking here than I have.
> 
> Yeah, I think it'd be pretty safe to go with a press-and-hold, similar to
> what Edge does.

What Edge does is different than what the native Windows form controls do, and I actually think that Edge's behavior is pretty crazy.  They only show the icon once and after you have used it, it goes away and there is no way to get it back AFAICT.  That is what I meant by designing the interaction.
I am not a fan of imposing browser UI on top of web content. Would a contextual menu help? A "Show password" toggle?
Flags: needinfo?(rfeeley)
(In reply to Ryan Feeley [:rfeeley] from comment #23)
> I am not a fan of imposing browser UI on top of web content. Would a
> contextual menu help? A "Show password" toggle?

The Show my Password extension does indeed operate from a pull-down context menu.  However, it is now a toggle.  Instead, the user selects the password dots or asterisks and then right-clicks to get the menu from which "Show password" can be selected.  

A drawback of the extension is that the password is exposed only temporarily, often not long enough.  Another drawback is that the "Show password" item in the context menu appears only on even-numbered selections of the password dots or asterisks; an unsteady hand can prevent this from working.
Attached image Hide/Hide with divider (obsolete) —
Some options for the contextual menu.
Attached image Show/hide no divider (obsolete) —
Attached image Hide/hide no divider (obsolete) —
(In reply to Ryan Feeley [:rfeeley] from comment #28)
> Created attachment 8680842 [details]
> Hide/hide no divider

I worry that the context menu will have low discoverability for the feature, but it will be a good v1. The 'no divider' option appears best to me, as the mockups with the dividers have a divider after almost every menuitem and it looks very zebra-stripey to me.
A good article about this topic by Luke Wroblewski.
http://www.lukew.com/ff/entry.asp?1941
I think the contextual menu is the right place for this. Putting browser UI on web content is problematic in so many cases (even though Apple now does it in Safari for passwords). I'm deferring to Stephen on this one, unless there are some big UX security issues I'm missing. The idea is that you can enable this per field and it is not saved for previous visits. Stephen, which above the above 4 options do you think works? (if any)
Flags: needinfo?(shorlander)
(In reply to Alejandro Moreno Calvo from comment #30)
> A good article about this topic by Luke Wroblewski.
> http://www.lukew.com/ff/entry.asp?1941

Love this article and referred to it heavily for password manager work, and even Firefox Accounts.
Note that if this ends up happening in a context menu rather than an in-textbox eyeball toggle/inline-whatever, then this means Firefox OS will likely need to do something different.  (Inactive/no-traction) bug 1111131 covers the Firefox OS case.  Additional context can be found in bug 1111131 comment 0 and in a dev.platform discussion thread at https://groups.google.com/forum/#!topic/mozilla.dev.platform/iNsK4egwRbI

For context, if you long-press in a password box on trunk Firefox OS, it activates a selection region with two handles and shows a hover menu with iconic buttons that represent "select all", "cut", and "copy".  (I suppose paste must show up in there sometimes too, but it's not happening for me.)  The icons are reasonably narrow, so I suppose an eyeball thing could be added there too.

Note that I'm not saying desktop and mobile have to do the same thing.  Just it would be nice ;)
See Also: → 1111131
Using a context menu is not very discoverable... because Twitter, Adobe etc. and even Microsoft Edge use an eye icon for this most users would be more than happy with that implementation.

If we are nervous about showing browserUI in content, which we arguably do with all controls, we could make it opt-in and / or customizable.

We could also have a small down arrow to show options a bit like Safari instead.

Of course, UX know best.
I'd be in favour of making password unmasking a browser preference, like pop-up blocking, that could also be controlled per site. Something for Control Center perhaps Aislinn?
Flags: needinfo?(agrigas)
(In reply to Ryan Feeley [:rfeeley] from comment #35)
> I'd be in favour of making password unmasking a browser preference, like
> pop-up blocking, that could also be controlled per site. Something for
> Control Center perhaps Aislinn?

I'm a bit confused at the problem we're trying to solve here. The original url when the bug was filed was pointing to evidence from Neilson that hiding passwords is a problem as 'users are typing'. This is a different use case than I think the end of this thread is talking about - revealing a password after it has been auto-filled for the user? 

Nonetheless, I don't think many users would discover a setting or inherently see the value or having auto-filled passwords revealed. Do we have any research showing people find it an issue outside of the manual password type in scenario?
Flags: needinfo?(agrigas)
It's a six year old old bug, but it's about giving users a way to unmask password fields on web pages like Edge does, regardless of where the passwords come from. It's ideal for someone living alone with a desktop computer that is secured. Masking passwords for this user offers little in terms of security.
Attachment #8680842 - Attachment is obsolete: true
Attachment #8680839 - Attachment is obsolete: true
Attachment #8680838 - Attachment is obsolete: true
Comment on attachment 8680841 [details]
Show/show no divider

Spoke with my UX team today and this is the desired design. Does not save per site, but that's something we can explore if it's received well.
Flags: needinfo?(shorlander)
Ehsan: here are some acceptance criteria to accompany the attached mockup.
1. When the user right-clicks the password field, Show Password item appears in contextual menu.
2. When the user chooses to Show Password, the password is unmasked, the contextual menu closes.
3. When the user reopens the contextual menu, a checkmark appears beside Show Password.
4. When the user chooses to no longer Show Password, the password is masked, and the contextual menu closes.

Make sense? Perhaps if this becomes useful, it can become managed like pop-up blocking permissions.
Flags: needinfo?(ehsan)
Oh, sorry I somehow missed this bug...  Thanks a lot for working on the UX design here!

Ryan, I'd like to take an opportunity to convince you against comment 23.  :-)  I believe that we can actually do a good job at embedding a browser-specific UI inside password fields, and as you probably know Edge and Safari have both successfully managed to ship such as UI.  That is also needed for native widget parity on Windows 10 (and also parity with our own browser UI which has been using similar UI in the password doorhanger prompts.)

Would you be available to discuss this in a meeting, or casually at the office at some point?
Flags: needinfo?(ehsan) → needinfo?(rfeeley)
Comment #39 looks good.  For security, the default should always be to "mask" the password on each newly rendered Web page, including pages retrieved from the cache.  That way, a password will not be inadvertently exposed if the user forgets the current state of "mask/unmask".  This will automatically ensure that the setting is always "mask" when launching the application anew.
(In reply to :Ehsan Akhgari from comment #40)
> 
> Ryan, I'd like to take an opportunity to convince you against comment 23. 
> :-)  I believe that we can actually do a good job at embedding a
> browser-specific UI inside password fields, and as you probably know Edge
> and Safari have both successfully managed to ship such as UI.  That is also
> needed for native widget parity on Windows 10 (and also parity with our own
> browser UI which has been using similar UI in the password doorhanger
> prompts.)
> 
> Would you be available to discuss this in a meeting, or casually at the
> office at some point?

Let's chat! I'm here always. Bring your convincing pills.
Flags: needinfo?(rfeeley)
See Also: → 1330228
The current NIST guidelines state that "memorized secret authenticators" (passwords and PINs) SHOULD be made visible through an option.

https://pages.nist.gov/800-63-3/sp800-63b.html#sec5
The UX underwent some a11y training this week and discovered that a11y professionals recommend at all sites include a password visibility toggle.

I think that we should approach this in two ways.

In the short term, we should implement the contextual menu item the team approved in comment 38.

https://bugzilla.mozilla.org/show_bug.cgi?id=502258#c38

In the longer term (and I'll need help specifying this) we should allow web developers to access this same visibilty toggle through their own front end code. Meaning that those who already have a show password toggle implemented can simply improve the security without having to change their design.

In the far longer term we could include a password visibility toggle icon of our own, but would need to be careful not to overlap the UI of the site.
See Also: → 1517593
Keywords: dupeme
Whiteboard: [DUPEME]
See Also: → 1548389
Blocks: 1604297

I raised this recently on Reddit, hoping to see this feature as the other major browsers have this and I really miss it!
https://reddit.com/r/firefox/comments/frpsp6/will_firefox_get_show_password_feature_like_other/

I tend to think we urgently need to implement this. There's clearly a user need for temporarily unmasking the text in password fields.

Attached image Edge screenshot 1

Here's the iconography in Edge on Win10, fwiw.

Attached image Edge screenshot 2

Do we already have SVGs for those symbols by any chance?

Flags: needinfo?(bgrinstead)
Attached patch WIP (obsolete) — Splinter Review

Here's a quick hack that seems to work to mask/unmask the value.
Just need SVG icons and add toggling between those...

Assignee: nobody → mats

I found these that looks good to me:
browser/components/aboutlogins/content/icons/password.svg
browser/components/aboutlogins/content/icons/password-hide.svg

Flags: needinfo?(bgrinstead)
Attached patch WIP (obsolete) — Splinter Review
Attachment #9248927 - Attachment is obsolete: true
Attachment #9248960 - Attachment is patch: true
Attachment #9248960 - Attachment mime type: application/octet-stream → text/plain
Attached video demo

Does this UI look OK so far?

Flags: needinfo?(bgrinstead)

The input probably shouldn't lose the focus ring after you click the button. FWIW I expect backlash from web developers unless the fill of the icon is the same as the text color (and we don't add any backgrounds, probably, or they're very subtle so that they don't alter the general design).

The input probably shouldn't lose the focus ring after you click the button.

Ah, good catch, I didn't even notice :-)

FWIW I expect backlash from web developers unless the fill of the icon is the same as the text color

Right, I haven't really looked at the SVGs at all so far. It appears background-color works fine, but it doesn't use color. I'll look into that, thanks!

I think the icon is using context-fill so it might just work if you add -moz-context-properties.

My other potential concern with landing something like this is that pages that provide their own toggle (like Google or such) will look out of sync, which is a bit unfortunate but maybe we just have to live with that?

Can we try to coordinate with other browsers on this or something?

I think the icon is using context-fill so it might just work if you add -moz-context-properties.

Thanks, I'll try that.

My other potential concern with landing something like this is that pages that provide their own toggle (like Google or such) will look out of sync

IMHO, sites that makes assumptions about what a specific UAs <input type=password> looks like are broken by design. Not that that stops web developers from still doing that of course, so I get what you're saying... :-)

maybe we just have to live with that?

I think so yeah, but I guess we can give a heads-up to major sites that we know are doing that.

Can we try to coordinate with other browsers on this or something?

I'm hoping the csswg issue I filed would spark some discussion / co-ordination. One way to get UAs to converge on this would be if the HTML spec would recommend that password fields have a built-in unmasking button, not normatively perhaps, but at least informatively as a good practice.

BTW, regarding the focus issue, given that I copied the styling from ::-moz-search-clear-button, I checked and it looks like <input type=search> has the same focus issue...

FWIW I expect backlash from web developers unless the fill of the icon is the same as the text color

FYI, if you try this in Edge on Win10: data:text/html,<input type=password style="background:black; color:red"> you'll se that Edge doesn't honor the color at all, so the password reveal button is invisible. Funny enough they also have the same problem with focus - when you click the reveal button the control loses focus, and not only that, if you click outside it and then focus it again and type more text the button never shows up again.
It looks like their control is even buggier than what I have so far...

BTW, -moz-context-properties: fill; fill: currentColor; fixed the color issue for me.
I'll look into the focus issue tomorrow...

Jamie, is there anything you would like regarding accessibility if we add this password reveal button?
Does Edge on Windows have some special a11y for their reveal button?

Flags: needinfo?(jteh)

Thanks for asking. Do I have to do anything special in Edge to get the password reveal button to show up? If I do:
data:text/html,<input type="password">
Even when I type something, I can't see the button appear anywhere in the accessibility tree. Either its a11y is busted or it isn't showing up at all. I can't confirm the latter as I don't have any sighted help at the moment.

In any case, if we're displaying a button visually, this should probably be exposed in the a11y tree as a separate labelled control outside of the input and it should also be tabbable. I imagine those things are going to be tricky to implement, though, because I'm guessing the anonymous button is created inside the text field (I don't know that code well enough to be sure).

Is this for desktop or both desktop and Android? If desktop, a context menu entry might be a suitable workaround for keyboard and screen reader users, albeit less discoverable. That's not going to work for Android, though.

Flags: needinfo?(jteh)

Thanks Mats for working on this! I like this solution a lot as it solves it for all users across the board.

I think it would be good to raise this at https://github.com/whatwg/html/issues for coordination with others as well as to indicate that password fields can contain such additional controls. Safari seems to have some custom UI inside password controls already, albeit it for password management. Chrome has nothing as far as I can tell.

Questions:

  • Does the state of the button take into account the computed value of input-security? It seems this would be important to not be out-of-sync with websites.
  • Does the state of the button affect the computed style input-security? E.g., if the user toggles it, does that change the computed value observable to the website? I'm not sure what the right behavior here is, but this might be something we have to specify as part of HTML integration.

If we decide to ship this I think it would be worth making some noise about it, e.g., with a blog post on Hacks. That way web developers have a heads up of sorts.

Keywords: dupemedev-doc-needed

(In reply to Anne (:annevk) from comment #64)

I think it would be good to raise this at https://github.com/whatwg/html/issues for coordination with others as well as to indicate that password fields can contain such additional controls.

Sure, I filed https://github.com/whatwg/html/issues/7293.

Safari seems to have some custom UI inside password controls already, albeit it for password management.

Yeah, it's kind of a dropdown menu; they could add a menu item for it there I suppose.

Chrome has nothing as far as I can tell.

I think someone mentioned they enable it in some situations / platforms... although I don't know details.
Given that Edge is now built on Chromium it should be easy for Chrome to add it, assuming Microsoft has/will upstream it.

  • Does the state of the button take into account the computed value of input-security?

No, I believe input-security is harmful to users and should be removed from the css-ui spec. I've filed https://github.com/w3c/csswg-drafts/issues/6788 about that.

If we decide to ship this I think it would be worth making some noise about it, e.g., with a blog post on Hacks. That way web developers have a heads up of sorts.

Yup, sounds good to me.

(In reply to James Teh [:Jamie] from comment #63)

Do I have to do anything special in Edge to get the password reveal button to show up?

Nope, just entering some text makes it show up. And deleting all text hides it again.
I'm copying that behavior, setting visibility:hidden when it's empty.

Even when I type something, I can't see the button appear anywhere in the accessibility tree.

OK, maybe they don't have a11y support for it then... let me know if you find out more later.
I'll try to ask someone working on Edge if I can figure out who to ask.

this should probably be exposed in the a11y tree as a separate labelled control outside of the input

Yeah, that sounds reasonable.

and it should also be tabbable.

I'm not sure that's possible to implement, or even desirable. If the reveal button is tabbable it will disrupt tabbing to the next control in the form, which I think will be surprising to most users. I suspect that's going to be regarded as a regression and not desirable.

I'm guessing the anonymous button is created inside the text field.

Correct, it's inside the text field.

Is this for desktop or both desktop and Android?

Both, I hope, although I haven't really tested it on any mobile platforms yet, so I'm not sure how well it works there...

If desktop, a context menu entry might be a suitable workaround for keyboard and screen reader users, albeit less discoverable.

Yes, I was planning on adding a context menu item and some keyboard command for it.

That's not going to work for Android, though.

Hmm, I thought we had context menus on Android too? (when you long tap it) (I could be wrong since I don't use Android)

(In reply to Mats Palmgren (:mats) from comment #55)

Does this UI look OK so far?

Thanks for sharing this. In addition to the working through a11y and potential compat issues I think we should connect with UX and product folks before landing anything. I'll make sure this gets on their radar.

Flags: needinfo?(bgrinstead)

Or landing behind a pref at least.

(In reply to Mats Palmgren (:mats) from comment #66)

and it should also be tabbable.

I'm not sure that's possible to implement, or even desirable. If the reveal button is tabbable it will disrupt tabbing to the next control in the form, which I think will be surprising to most users. I suspect that's going to be regarded as a regression and not desirable.

Perhaps... but on the other hand, having it not tabbable means it can't be discovered/accessed as easily by keyboard users. Yes, we could have a keyboard shortcut or a context menu entry, but that's not particularly discoverable. I assume the whole reason we're doing this as a button instead of a context menu entry for mouse users is discoverability/ease of access.

I'm guessing the anonymous button is created inside the text field.

Correct, it's inside the text field.

That's going to be a problem for a11y and is possibly why Edge doesn't expose it. A11y needs this to be outside of the text field. I guess we could try to hack around that in the a11y engine, but I'm not sure how yet.

Hmm, I thought we had context menus on Android too? (when you long tap it) (I could be wrong since I don't use Android)

You might well be right. I don't use Android either. :)

Google folks said they got several bug reports when they tried to show a button like this by default. The issue being that web sites have a similar functionality already and then there might be two different looking buttons to do basically the same thing.

(In reply to Brian Grinstead [:bgrins] from comment #67)

(In reply to Mats Palmgren (:mats) from comment #55)

Does this UI look OK so far?

Thanks for sharing this. In addition to the working through a11y and potential compat issues I think we should connect with UX and product folks before landing anything. I'll make sure this gets on their radar.

As per product folks, this would be a good component to a larger revamp of the passwords and login experience - which is on the radar but not currently prioritized. Given potential compat issues (Comment 70) and a11y engineering work (Comment 69) I'll suggest that enabling this by default stays on the backlog until then.

This bug report is "well in hand". I am removing myself from receiving further E-mail about it.

(In reply to James Teh [:Jamie] from comment #69)

That's going to be a problem for a11y and is possibly why Edge doesn't expose it.

OK, I added aria-hidden=true to hide it. Turns out we already do that for the up/down buttons inside number controls:
https://searchfox.org/mozilla-central/rev/a12c2c2e59c92d8f969d8f3f290ab16919449c9d/layout/forms/nsTextControlFrame.cpp#370

(I guess we should probably do that to the Clear button inside <input type=search> too -- I'll file a separate bug about that)

(In reply to Olli Pettay [:smaug] from comment #70)

Google folks said they got several bug reports when they tried to show a button like this by default.

Well, Edge has this button so I think it should be fine for us to ship it too.

It's controlled by the pref:
layout.forms.input-type-show-password-button.enabled

Enable it by default in Nightly for testing purposes.

Attachment #9248960 - Attachment is obsolete: true

(In reply to Mats Palmgren (:mats) from comment #73)

OK, I added aria-hidden=true to hide it. Turns out we already do that for the up/down buttons inside number controls:

While I'm struggling to come up with another solution here, I'd point out that the up/down buttons for number controls and the clear search button all have very intuitive keyboard equivalents: up/down arrows and escape (or control+a then delete), respectively. Hiding the show password button makes the feature inaccessible to screen reader users. Having it not tabbable makes it inaccessible to keyboard users. Adding a context menu entry makes it accessible, but not discoverable, which means it is less accessible to keyboard/screen reader users than it is to mouse users (by virtue of it not being easily discoverable).

All this is to say that while landing this behind a pref is fine, these issues need to be sorted out properly before we even consider shipping it. Otherwise, we'll have the same situation we have with native video controls: authors will continue to rely on custom implementations because native web browser implementations have serious a11y problems.

(In reply to Mats Palmgren (:mats) from comment #66)

and it should also be tabbable.

I'm not sure that's possible to implement, or even desirable. If the reveal button is tabbable it will disrupt tabbing to the next control in the form, which I think will be surprising to most users.

Will it, though? Custom implementations of the reveal password button are probably tabbable, unless authors explicitly set tabindex="-1". And why would they do that? If there's a control discoverable to mouse users, why shouldn't it be similarly discoverable by keyboard users?

(ni? emilio for help fixing the context menu item)

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)

I think this is nicer, should persist the state across reframes, and is
less special.

input/textarea tests in browser_contextmenu.js are disabled :-(

Depends on D130661

Attachment #9249316 - Attachment description: Bug 502258 - Add a Show Password button to <input type=password> controls. r=emilio → Bug 502258 part 1 - Add a Show Password button to <input type=password> controls. r=emilio,mats

Comment on attachment 9249808 [details]
Bug 502258 - Simplify implementation of "show password" / "clear" button. r=mats

(folded into part 1)

Attachment #9249808 - Attachment is obsolete: true
Attachment #9249809 - Attachment description: Bug 502258 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs! → Bug 502258 - part 2 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs!

(In reply to James Teh [:Jamie] from comment #76)

I'd point out that the up/down buttons for number controls and the clear search button all have very intuitive keyboard equivalents: up/down arrows and escape (or control+a then delete), respectively.

True, but please note that neither of the keyboard commands you mentioned are discoverable at all. A user must have learned them beforehand to know that they can use them. (Granted, keyboard users most likely are familiar with them, just saying that they aren't discoverable.)

Adding a context menu entry makes it accessible, but not discoverable

I disagree, a context menu item is discoverable. Less so than a focusable control, but still more discoverable than the keyboard commands mentioned above, which are not present in the context menu (although we might want to add a "Clear" context menu item for search input elements). We can add an accelerator key for it too, if that's desirable. I'll leave that for UX folks to decide, but that shouldn't block landing this in Nightly for testing purposes.

(In reply to Mats Palmgren (:mats) from comment #82)

I'd point out that the up/down buttons for number controls and the clear search button all have very intuitive keyboard equivalents: up/down arrows and escape (or control+a then delete), respectively.

True, but please note that neither of the keyboard commands you mentioned are discoverable at all. A user must have learned them beforehand to know that they can use them. (Granted, keyboard users most likely are familiar with them, just saying that they aren't discoverable.)

That's fair. I guess I'm arguing that prior familiarity with established conventions/patterns is an acceptable substitute for discoverability here. In contrast, there isn't really an established convention/pattern for keyboard access to reveal a password (except tabbing to a button).

I disagree, a context menu item is discoverable. Less so than a focusable control

Right; I should have said "less discoverable". My argument is: if we see the need to make it more clearly discoverable for mouse users (who could also use the context menu), that suggests we think it's not sufficiently discoverable for them. In that case, we have to ask why the same logic doesn't apply for keyboard users.

(In reply to James Teh [:Jamie] from comment #83)

In that case, we have to ask why the same logic doesn't apply for keyboard users.

That's fair, but I just don't see a way to do that, besides the context menu item and possibly adding an accelerator key. I don't think making this button focusable is an option. Both because it will likely require major surgery to implement (might be impossible actually), and secondly because it interrupts tabbing to the next form control which I suspect will be surprising and undesirable new behavior to many users. It's worth noting that both Edge and Safari have a button inside their password control and they chose to not make it focusable. All things considered, I think that's the right trade-off for us too.

That said, I don't feel strongly about the focusability, so if someone can supply patches that implements that in a reasonable way that's fine with me. I don't see that as blocking landing (and enabling) this in Nightly though.

BTW, for the screen reader case, a11y has access to the DOM so it seems to me it could easily retrieve the password in clear text and present that to AT through some function if we want.

(In reply to Mats Palmgren (:mats) from comment #84)

In that case, we have to ask why the same logic doesn't apply for keyboard users.

That's fair, but I just don't see a way to do that, besides the context menu item and possibly adding an accelerator key. I don't think making this button focusable is an option. Both because it will likely require major surgery to implement (might be impossible actually), and secondly because it interrupts tabbing to the next form control which I suspect will be surprising and undesirable new behavior to many users.

Perhaps, but you could argue having the control visible at all will be surprising new behaviour. Also, this is what already happens with at least some custom reveal password buttons.

It's worth noting that both Edge and Safari have a button inside their password control and they chose to not make it focusable. All things considered, I think that's the right trade-off for us too.

Their choices don't necessarily mean that's what's best for users. There are multiple articles discussing accessibility issues in native HTML widgets and how these are often why authors end up choosing custom implementations over native ones. I worry this will be yet another example of that.

Attachment #9249316 - Attachment description: Bug 502258 part 1 - Add a Show Password button to <input type=password> controls. r=emilio,mats → Bug 502258 - Add a Show Password button to <input type=password> controls. r=emilio,mats

Comment on attachment 9249809 [details]
Bug 502258 - part 2 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs!

(I merged the two parts.)

Attachment #9249809 - Attachment description: Bug 502258 - part 2 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs! → Bug 502258 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs!
Attachment #9249809 - Attachment is obsolete: true
Attachment #9249809 - Attachment description: Bug 502258 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs! → Bug 502258 - part 2 - Implement "Toggle show password" context menu correctly. r=mats!,Gijs!
Attachment #9249809 - Attachment is obsolete: false
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/68ad8a7fd3cc
Add a Show Password button to <input type=password> controls.  r=Gijs
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31748 for changes under testing/web-platform/tests
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/31d134fa6717
Don't enable the pref explicitly in context menu test.
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
Upstream PR merged by moz-wptsync-bot
QA Whiteboard: [qa-96b-p2]

FF96 docs work for this in https://github.com/mdn/content/issues/10855. Essentially it is just an update to the Experimental firefox features page to reveal the preference, and a minor update to HTML/Element/input/password to note this behaviour as another possible browser implementation behaviour.

I think it is too early to do much more - e.g. browser compatibility data updates, until the discussion proceeds to a more concrete specification in https://github.com/whatwg/html/issues/7293

You need to log in before you can comment on or make changes to this bug.