Open Bug 259059 Opened 20 years ago Updated 1 year ago

Show when CAPS-LOCK is on and focus is in a password field

Categories

(Core :: Layout: Form Controls, enhancement)

Product:
Core
Component:
Layout: Form Controls
Type:
enhancement

Tracking

()

People

(Reporter: glazou, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: polish)

Attachments

(3 files, 12 obsolete files)

Screenshot #2 (Patch v1.1)
8.30 KB, image/png
Details
Patch v2.0 pre #2
71.38 KB, patch
Details | Diff | Splinter Review
suggested UI
25.14 KB, image/jpeg
Details 
I suggest to add a visible hint if the caps-lock key is "on" when the focus
is on a password field, on a HTTP/FTP account/password popup, or even the
Master Password query popup.
Good idea, why don't add the num lock key ?
Note that it should fit the OS hint (i.e. Tooltip on Windows, Inline icon on 
mac).
QA Contact: firefox.general → bugs.mano
-> taking in efforts to win a prize in the extend firefox competition...  Will start out as an extension, but will be GPL'd so you can incorporate it into the trunk if you wish.
Status: NEW → ASSIGNED
Assignee: firefox → mikegoodspeed
Status: ASSIGNED → NEW
As far as I can tell, there's no way to determine the CAPS_LOCK status in C++ (it's platform specific), JavaScript, or XUL.  I might look into compiling per-platform dlls.
I can't get this extension to work on Firefox 1.5RC1.  Anything I can do about that?

I do see that you used compiled code, though.  Your screenshot shows it in the master password dialog box.  Does it work on document password fields too?  
Mike, for the record, if you license as GPL-only we can't check it into CVS, since we use the MPL/GPL/LGPL tri-license.
Yes, I realized that afterwards and knew that I needed to do it that way, but didn't feel like bugspamming everyone about it.  I guess I am now!
Flags: blocking1.9a1?
I can't change the flag fields, but this isn't at all ready for actual checkin.  There is no patch to the trunk.  The extension is for Windows only (needs to work on Win/Lin/OSX).  The chrome it uses to notify the user is an initial test.
(In reply to comment #9)
> I can't change the flag fields, but this isn't at all ready for actual checkin.
>  There is no patch to the trunk.  The extension is for Windows only (needs to
> work on Win/Lin/OSX).  The chrome it uses to notify the user is an initial
> test.

Agreed 100%. Digger_Rock@arcor.de, you shouldn't have marked the bug.
AFAIK, there is no way to arbitrarily check the status of the capslock key
outside of a key event callback on X11. This implies the solution may remain
windows-only... In such case, an extension (as it is today) is probably the right
way to process.
Flags: blocking1.9a1?
Assignee: mikegoodspeed → nobody
QA Contact: bugs.mano → general
*** Bug 353327 has been marked as a duplicate of this bug. ***
*** Bug 354779 has been marked as a duplicate of this bug. ***
On, XP, doing this "natively" requires using a balloon tooltip, which I don't think we currently support.  I don't know what UI Vista uses for the caps lock warning.
Keywords: polish
There is a way to check for caps lock in Javascript. Just check whether a capital letter was sent under window.event.keyCode and the Shift Key was NOT pressed (window.event.shiftKey), or whether a small letter was passed and the Shift Key is pressed.

It may be an ugly way but it's a way. :)
(In reply to comment #14)
> There is a way to check for caps lock in Javascript. Just check whether a
> capital letter was sent under window.event.keyCode and the Shift Key was NOT
> pressed (window.event.shiftKey), or whether a small letter was passed and the
> Shift Key is pressed.
> 
> It may be an ugly way but it's a way. :)

The tooltip has to show up as soon as caps lock is pressed, and has to go away as soon as caps lock is turned off, to act like native XP password inputs.
Blocks: 362580
I filed bug 377478 that is a back end part for this bug. And I attached the proposal patch for it. But the patch is hacky especially on Mac. (Probably, only on Win32, the patch is not hacky.) Therefore, I want some advices and testers for it.
I fixed bug 377478, so, we can fix this bug, now if somebody can create the front-end patch.

Note that bug 377478 did *not* change any IDL files, so, we cannot use the new function from Javascript, but you can create the new method to an interface easily. So the new method only calls the nsIKBStateControl::GetToggledKeyState. But I don't know which object should have the method. (therefore, I didn't change the any IDL files.)

You can see the example for GetToggledKeyState:
https://bugzilla.mozilla.org/attachment.cgi?id=268321&action=diff#mozilla/editor/libeditor/base/nsEditor.cpp_sec1
Would it make sense to do this at the Toolkit level so Thunderbird can benefit from the fix?
I have an idea to fix this bug.

nsTextControlFrame or editor should open a popup window via nsIWindowWatcher like nsAlertService, and the URL of the popup should be chrome://global/content/keyboardStatePopup.xul or something. And the XUL file will be defined in toolkit/content. So, my plan can fix this bug in Core/Toolkit level.
Status: NEW → ASSIGNED
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
Assignee: nobody → masayuki
Attached image screenshot #1 (obsolete) — 
The first draft patch works fine for me. I need more works, but the plan is correctly. I'll post a draft patch in some days.
(In reply to comment #21)

> The first draft patch works fine for me. I need more works, but the plan is
> correctly. I'll post a draft patch in some days.

Very nice. Don't use the same background-color as the main prompt window?
Please fix the typos in the patch/screenshot too:
s/passowrd/password/
s/trun/turn/
(In reply to comment #21)
> Created an attachment (id=339758) [details]
> screenshot #1

Shouldn't the text be part of the dialog instead of a box hovering over it? I'd suggest to:
1. expand/enlarge the dialog downward
2. place the text below the PW field and above the OK button
3. perhaps make the background color pale-yellow (like other warnings/alerts)

Shouldn't the text to the right of the triangle be vertically centered on the triangle?

Make it idiot-prof: ... press the Caps Loc key on your keyboard...?

On some settings (mine!), pressing Caps Loc does NOT exit caps loc mode. I have to press the Caps (SHIFT) key (arrow pointing UP). "Exit Caps Loc mode by pressing either the Caps Loc key or SHIFT key, depending on your setting." or  "Exit Caps Loc mode to make this message disappear."
Peter:

No, my patch makes floating tooltip from password field. The alert is not appeared only on the authorizing prompt. So, it is also appeared on the password fields on web pages.

The message in the screenshot is same as IE's. I'll change the text in first draft patch. Because maybe we don't use the alert only for Caps Lock state. It may be usable for Insert/Overwriting mode alert on our editors.
Attached patch Patch v1.0 (obsolete) — — Splinter Review 
This works fine. However, the performance is not good. Because this creates a window for the tooltip owner. It's cause the mouse cursor becoming "processing". I'm trying to think another way. E.g., the window is created at first time only, or finding the way which we can create a tooltip directly.
Attachment #339758 - Attachment is obsolete: true
Attached patch Patch v1.0.1 (obsolete) — — Splinter Review 
missing a properties file.
Attachment #340162 - Attachment is obsolete: true
I find bug 399437. If you click the alert balloon, the alert is closed, but the caret will be invisible until re-activate the window. Then, the selection by mouse is also broken.
Attached patch Patch v1.1 (obsolete) — — Splinter Review 
fix up some issues.
Attachment #340190 - Attachment is obsolete: true
Comment on attachment 340315 [details] [diff] [review]
Patch v1.1

>+#define ERROR_ICON_24_URL \
>+          NS_LITERAL_STRING("chrome://global/skin/icons/error-16.png")
>+#define INFORMATION_ICON_24_URL \
>+          NS_LITERAL_STRING("chrome://global/skin/icons/information-16.png")
>+#define QUESTION_ICON_24_URL \
>+          NS_LITERAL_STRING("chrome://global/skin/icons/question-16.png")
>+#define WARNING_ICON_24_URL \
>+          NS_LITERAL_STRING("chrome://global/skin/icons/warning-16.png")

Hardcoding icons like this breaks using stock icons on Linux. :(
Attached patch Patch v1.2 (obsolete) — — Splinter Review 
Thank you, Reed.
Attachment #340315 - Attachment is obsolete: true
Attached patch Patch v1.3 (obsolete) — — Splinter Review 

        Attachment #340711 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1.3.1 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #340743 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1.3.2 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #340747 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1.3.3 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #340750 -
        Attachment is obsolete: true

    
    

    
  
Note that I don't want to fix Mac style (using backgroun-image) in this bug. Because it needs some additional works, it should be separated to another bug after this.

Also, I don't want to use the balloon style for Windows and Linux in this bug. Because it needs many works and I cannot implement it. So, it should be separated to another bug and someone should fix it.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1.4 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #340771 -
        Attachment is obsolete: true

    
    

    
  
Now, I'm working for:

* making simpler interface.
* embedded browsers can handle the showing/hiding timing via nsIPopupListener (like nsIToolkitListener).
* but the popup showing/hiding methods should be scriptable for automated testing. (and it may help some extension developers.)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v2.0 pre #1 (obsolete)
        — 
          — Splinter Review
      

    


  
This is new structure patch. Note that this doesn't work, it's in progress. The backend part (C++ part) is finished most works, but I need to rewrite the frontend pard (XUL part) for the new structure.

The client can use nsIPopupService for showing popup. The nsPopupService leaves the actual behavior to nsPopupServiceManager which is singleton and it keeps the all popuped objects/timers for auto hidden/popup listeners. nsPopupServiceManager notify the popup showing/hiding event to nsIPopupListener. It is implemented with nsIDocShellTreeOwner. Therefore, any embedded browser developers can implement the popup without XUL.

        Attachment #340779 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v2.0 pre #1 (obsolete)
        — 
          — Splinter Review
      

    


  
adding a missing file (nsIPopupListener.idl)

        Attachment #342447 -
        Attachment is obsolete: true

    
    

    
  
I see, yet another issue that's been around for years and has patches being left to rot.

(In reply to comment #1)
> Good idea, why don't add the num lock key ?

Would be valuable for laptop users, but probably annoying for those using typical desktop computer keyboards.  But can we detect what type of keyboard is being used?  There's even the possibility that the system has two keyboards - for instance, in my company each of us uses a laptop with a wireless keyboard added.

Maybe the best we can do is a pref....

    
    

    
  
Ok, let's try to revive this bug. I think it's useful wherever you have a password field, either on the web or in chrome.

What about using a doorhanger panel to display this message? Now that we have such a nice thing and will be using it to display mismatching input of html5 forms anyways, I think we should use them here for consistency.

Will we get into troubles that the next keystrokes will hide the doorhanger notification?

    
    

    
  
(In reply to comment #50)
> What about using a doorhanger panel to display this message? Now that we have
> such a nice thing and will be using it to display mismatching input of html5
> forms anyways, I think we should use them here for consistency.

If the panel isn't clipped by the window, i.e., it's an independent panel, it's useful for Windows.

The native popup of Win7 is different from Vista or earlier, therefore, I think that we don't need to emulate the native look and feel now.

> Will we get into troubles that the next keystrokes will hide the doorhanger
> notification?

I think so. While CapsLock is locked, the notification should be displayed always.

Unfortunately, I don't have much time now. If you want to fix this bug on Fx4, please fix it yourself. The main code for this bug is only nsTextControlFrame part of the latest patch. It's very simple.

    
    

    
  
Using the arrow panels (aka. doorhangers) is a fine use of this to indicate Caps Lock in the password field.

It's already aware of window positioning, and will never render off screen.

    
    

    
  
I'd really like to contribute here but I have important work that is due before christmas. I won't be able to have a look at this until then.

    
    

    
  

      
      
      
      

        Attached image
          
        suggested UI
        — 
      

    


  

    
    

    
  
Masayuki, any chance revisiting this bug? 

The UI should be very unintrusive, like Mac does it (like in Comment 54). So no doorhangers or alerts or popups or anything, that'll just interrupt users. Some people actually use the CAPSLOCK regularly, especially in password fields (though I don't get this …). So please, only show an arrow icon like Mac does. 

Of course, native UI should be preferred, however I think just providing a common solution and override it for specific platforms is the most sensible approach here. 

Also, this should be constrained to input[type=text|password|search|email|url] (prior patches likely need to be updated to support some new HTML5 input types).

    
    

    
  
Would be there a preference to turn this feature off?
Because on some tastaure Layouts some characters are only aviable with CapsLock, so it could be negative if you type your password in and everybody near you can see that you use this characters...

    
  
Blocks: 941867

    
  
See Also:  → 953820

    
    

    
  
I'm still not sure how do I implement the UI feature of this bug.

However, there are a lot of requests of fixing this. It might be better I to implement it only backend side. Then, somebody would implement this on UI side.

Smaug, I think that we should fire customized events on text editable element only in chrome when UI should notify users of the state. Then, UI developers or addon developers can implement the UI with handling the events. How do you think?
Flags: needinfo?(bugs)

    
    

    
  
Could we add a way for chrome to check whether caps-lock is on. Then chrome code could add
a focus listener and in case pw field is focused when caps-lock is on, the UI code would show a warning.
Flags: needinfo?(bugs)

    
    

    
  
(In reply to Olli Pettay [:smaug] from comment #59)
> Could we add a way for chrome to check whether caps-lock is on. Then chrome
> code could add
> a focus listener and in case pw field is focused when caps-lock is on, the
> UI code would show a warning.

Yeah, it's possible. Currently, nsIWidget has the API but cannot access it from JS. However, currently, D3E is available.

1. UI can check CapsLock key event and getModifierState("CapsLock").
2. UI should handle focus and blur events with new API.

Then, we can fix this bug. I'll implement #2 in new bug. Do you think nsIDOMWindowUtils is good interface to have the API? I think same as KeyboardEvent, getModifierState(modifierKeyName) is useful.

I'm not sure about addon SDK. Can we add an API to it too? I worry about that if your approach is available on e10s.
Flags: needinfo?(bugs)

    
    

    
  
in e10s child process can send message to parent when password field is focused and parent process
then check if CapsLock is on, and show some warning.

I don't understand what you mean with #2.

getModifierState in DOMWindowUtils would be good, assuming it doesn't need to synchronously
ask some data from OS.
Flags: needinfo?(bugs)

    
    

    
  
I would like to indicate again Comment 57. This feature must be implemented behind a pref (maybe enabled by default). I like the layout on attachment 617161 [details], because you have an option right there to disable the warnings.

    
    

    
  
(In reply to Olli Pettay [:smaug] from comment #61)
> I don't understand what you mean with #2.

When a password field gets focus, UI needs to check CapsLock state for initializing notification UI. And also when a password field loses focus, UI needs to remove the notification UI.

> getModifierState in DOMWindowUtils would be good, assuming it doesn't need
> to synchronously
> ask some data from OS.

Did you mean ESM or something should store the last modifier state and nsIDOMWindowUtils::getModifierState() should use it? (Rather than asking it to OS)

    
    

    
  
Whilst we are waiting for a suitable fix which sounds like it is not as simple as it initially was thought to be and also variable between OS, is it not possible to simply add some text to the pop up password entry window saying something like:

'Passwords are case sensitive, check 'Caps Lock' status before entering password.'


It does not meet the request for the 'Caps Lock' warning  to be only triggered when 'Caps Lock' is enabled.

But it will prompt users to check before entering password.

So it is really just a reminder to jog the memory to check the current status of a 'Caps Lock'.

This does not depend upon any special checking codes and should be fast to implement.


Is there any reason why this workaround cannot be implemented to be included in the next release?



    
    

    
  
(In reply to Anje from comment #65)




Whilst we are waiting for a suitable fix which sounds like it is not as simple as it initially was thought to be and also variable between OS,




Of course it would be.  The mechanism for detecting whether caps lock is on naturally isn't going to be the same across all platforms.  Moreover, all GUI applications make use of OS-specific APIs, either directly or indirectly.




is it not possible to simply add some text to the pop up password entry window saying something like:

'Passwords are case sensitive, check 'Caps Lock' status before entering password.'




Hmm.  Passwords are usually case-sensitive but aren't always.  The browser has no way of knowing.  It's also possible that the input is for a purely numeric passcode/PIN, in which case a message like this would look a bit silly.



    
    

    
  
re: The mechanism for detecting whether caps lock is on naturally isn't going to be the same across all platforms.

I know, hence why it was mentioned and possible reason for delays in resolution.


re : Passwords are usually case-sensitive but aren't always. The browser has no way of knowing.

I'm talking about Thunderbird email client not Firefox Browser.

Maybe I'm not using the correct bug report or is this bug report something that is pertinent to both?


Perhaps changing the wording to satisfy issue.

eg: 'Passwords may be case sensitive, check 'Caps Lock' status before entering password.'


This does not mean switch off Caps Lock it means check it's status as it may effect the password you are entering.



    
    

    
  
(In reply to Anje from comment #67)




I'm talking about Thunderbird email client not Firefox Browser.

Maybe I'm not using the correct bug report or is this bug report something that is pertinent to both?




It would be equally relevant to both. I just naturally assumed it to be talking about the browser, but see now that it's filed against Core and is about password fields generally, not any particular kind of them.  So it looks to me like you've come to the right place.



    
    

    
  
Resetting assignee which I don't work on in this several months.


Assignee: masayuki → nobody
Status: ASSIGNED → NEW

    
    

    
  
This looks to be a very useful function, as it is very easy for busy, or non-tech users to not realise that CAP-LOCK is on (some keyboards have very small LED lights, and it is not always obvious). Also, some users might keep retrying, until the login attempts are blocked (thinking that they have mis-typed), then have to go through the hassle of site dialogue, reset passwod etc.).


It goes without saying, that this would ONLY be an issue if the password was NOT IN the LockWise password manager. However, some users don't always use it, and may have specific sites that they do NOT want to use it for.


So, is it possible to revisit this, or pass to a team where some related code or enhancement is being worked on?


Flags: needinfo?(masayuki)

    
    

    
  
Currently, UI events such as KeyboardEvent, MouseEvent etc has GetModifierState method. Therefore, at least, when user interaction occurs, content process can know whether CapsLock is "on" or "off". For cleaner implementation, focus event should have the information internally, but I think that it's not urgent. So in my area, I don't think that I should work this on anymore for now. The remaining issue is, how to let users know whether CapsLock is "on" or "off". I was thinking that it should be a tooltip which is introduced in Win2K's native UI and XUL theme can modify it, but that's what I couldn't solved. So layout or toolkit developers can implement enough UI to let users know the state, this can be fixed in most cases.


Flags: needinfo?(masayuki)

    
  
Severity: normal → S3
Component: General → Layout: Form Controls

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: