Bug 394107 added the methods 5 years ago. However, other platforms don't use them. And now, nsIWidget::OnIMEFocusChange() is always called when our editable editor gets or loses focus. So, cocoa widget can switch the secure event input mode in it instead of nsIWidget::BeginSecureKeyboardInput() and nsIWidget::EndSecureKeyboardInput().

I'm thinking about adding automated tests for this if it's possible.

oops...

roc: Can we dispatch custom DOM event from widget? If so, the test would be simple. We can fire custom DOM events on the DOM window when the secure input mode is changed. If it's impossible, should I made a new widget event which would cause firing DOM custom event? Or, do you have better idea?

Ah, MOZ_ASSERT() in the patch could be useful for the test.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3) > Can we dispatch custom DOM event from widget? If so, the test would be > simple. We can fire custom DOM events on the DOM window when the secure > input mode is changed. I don't think we should fire custom DOM events from widget.

These methods are only used on Mac. And current input context has the information if the focused element is password field or not. Therefore, these methods are not necessary.

This patch manages the secure input mode at setting input context and changing focus. On current trunk build, we don't exit from secure input mode if a password field has focus and we're deactivated. Then, the secure input mode is enabled on all other applications. This patch fixes this bug too.

Adding tests for this. However, unfortunately, the bug mentioned in the previous comment cannot test with our test framework, though.

Comment on attachment 752758 [details] [diff] [review] part.2 Cocoa widget should manage secure input mode with input context at changing the context and changing focus This basically looks fine to me. But I'm puzzled that you have to implement NotifyIME() and SetInputContext() in nsCocoaWindow (in addition to nsChildView). As I understand it, text input (of any kind) can only take place when an nsChildView is focused, so why pay attention to calls that focus or unfocus an nsCocoaWindow object? Also, some of the comments need to be clarified: + /** + * EnableSecureEventInput() and DisableSecureEventInput() warp the same + * name APIs. They manage count of calls in our process. + * API's IsSecureEventInputEnabled() returns if the mode is enabled at + * that time, whereas this class's IsSecureEventInputEnabled() returns + * if we've made secure event input mode enabled. + */ This should be something like: EnableSecureEventInput() and DisableSecureEventInput() wrap the Carbon Event Manager APIs with the same names. In addition they keep track of how many times we've called them (in the same process) -- unlike the Carbon Event Manager APIs, which only keep track of how many times they've been called from any and all processes. The Carbon Event Manager's IsSecureEventInputEnabled() returns whether secure event input mode is enabled (in any process). This class's IsSecureEventInputEnabled() returns whether we've made any calls to EnableSecureEventInput() that are not (yet) offset by the calls we've made to DisableSecureEventInput(). + /** + * EnsrueSecureEventInputDisabled() calls DisableSecureEventInput() until + * our call count becomes 0. + */ Ensrue... -> Ensure + NS_ASSERTION(!!sSecureEventInputCount == !!::IsSecureEventInputEnabled(), + "Something other process(es) has made secure event input enabled"); The comment should be something like: "Some other process has enabled secure event input"

Comment on attachment 752760 [details] [diff] [review] part.3 Add tests for secure input mode of Cocoa widget This looks fine to me. But of course "Not crahsed" should be "Not crashed" :-)

Thank you! https://hg.mozilla.org/integration/mozilla-inbound/rev/c4c6bb40c217 https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e3eb662e4e https://hg.mozilla.org/integration/mozilla-inbound/rev/53afe9fba5c0 https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=e26d50054025

https://hg.mozilla.org/mozilla-central/rev/c4c6bb40c217 https://hg.mozilla.org/mozilla-central/rev/b6e3eb662e4e https://hg.mozilla.org/mozilla-central/rev/53afe9fba5c0