Closed Bug 582893 Opened 11 years ago Closed 11 years ago
IME isn't disabled when password fields on sheet dialog get focus
This is a regression of bug 513952. nsCocoaIMEHandler::IsFocused() checks [window isMainWindow], however, it should be |[window isMainWindow] || [window isSheet]|. I'll post the patch.
Summary: IME isn't disabled when password fields on sheet dialog gets focus → IME isn't disabled when password fields on sheet dialog get focus
The main cause is [NSWindow isSheet] isn't called from IsFocus(). And there are two additional issues. One is ::TSMGetActiveDocument() returns *latest* active document. On 10.6, the result of ::TSMGetActiveDocument() is same pointer with the result of [NSInputManager currentInputManager]. [NSInputManager currentInputManager] returns *current* instance. So, we must call it before we call ::TSMGetActiveDocument(), then, we can get the right active document during focus changing. The other issue is, ResetTimer() quits if mTimer is already created but it cancels the current timer. This is a bug, it should reboot the timer. Note that this patch has a test. However, it can *not* detect this bug because nsChildView::GetIMEEnabled() returns non-actual state of the system. If we test this regression, we need to add an API to get the actual IME state of the system. But it's not good for now, it should be tested on native IME code testing (bug 460056). But I think the new test may be worth for some other regressions.
Sorry, this is "-U 8 -p".
Comment on attachment 462014 [details] [diff] [review] Patch v1.0 This looks fine to me. But I haven't tested it (or compiled it), because I don't have a testcase I can use "by hand". Please provide one, if possible. (From comment #0) > One is ::TSMGetActiveDocument() returns *latest* active document. On > 10.6, the result of ::TSMGetActiveDocument() is same pointer with > the result of [NSInputManager currentInputManager]. [NSInputManager > currentInputManager] returns *current* instance. This is false ... or at least unclear. But your comment in nsCocoaIMEHandler::GetCurrentTSMDocumentID() is much better: + // First, we must call [NSInputManager currentInputManager] because if + // focus is moving, this calling changes the result of + // ::TSMGetActiveDocument() from old focused window's to new focused window's. The gist of it is that (on OS X 10.6.X at least) ::TSMGetActiveDocument() has a bug that prevents it from returning accurate results unless [NSInputManager currentInputManager] is called first. I didn't know this (and without a testcase I'm unable to confirm it). But it certainly sounds plausible :-)
Attachment #462014 - Flags: review?(smichaud) → review+
Thank you, Steven. (In reply to comment #3) > This looks fine to me. But I haven't tested it (or compiled it), > because I don't have a testcase I can use "by hand". Please provide > one, if possible. Paste following code to the input field of _Error Console_. > var arg1 = new Object(), arg2 = new Object(); Components.classes["@mozilla.org/embedcomp/prompt-service;1"].getService(Components.interfaces.nsIPromptService).promptPassword(window, "title", "text", arg1, "msg", arg2); Then, you can see this bug. > (From comment #0) > > > One is ::TSMGetActiveDocument() returns *latest* active document. On > > 10.6, the result of ::TSMGetActiveDocument() is same pointer with > > the result of [NSInputManager currentInputManager]. [NSInputManager > > currentInputManager] returns *current* instance. > > This is false ... or at least unclear. Ah, I meant that the TSMDocumentID is an alias of NSInputManager on 10.6. However, the currentInputManger returns an actual *current* input manager, but the ::TSMGetActiveDocument() isn't looking for the actual current input manager itself during moving focus. So, it returns an NSInputManger of previous focused view. First, I just added [window isSheet] to IsFocused(), but the SyncASCIICapableOnly() didn't work fine. I was confused by that.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Ugh, something was changed on trunk. The new test was now disabled temporary. http://hg.mozilla.org/mozilla-central/rev/68b886f9b3c3
um... I cannot reproduce the failure on my system... 13908 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/widget/tests/test_imestate.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]" nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)" location: "JS frame :: chrome://mochikit/content/chrome/widget/tests/test_imestate.html :: runTestPasswordFieldOnDialog :: line 905" data: no] at :0
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.