test_imestate testing for MozIMEFocusOut is broken

RESOLVED FIXED in mozilla22

Status

()

Core
Widget
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bz, Assigned: masayuki)

Tracking

Trunk
mozilla22
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The test calls is_not() inside the "function onFocus" in the MozIMEFocusOut case of the switch, but there is no such function.  So that just throws an exception, which is never reported due to bug 503244 and hence the test ends up passing.

If I fix the test to call isnot(), then I get test failures.  If I comment out the entire call, the test is green but of course not testing what it presumably means to be testing...

Comment 1

5 years ago
Masayuki, could you look at this?
Yep, probably, I will have some time for this next week.
Masayuki-san, have you had a chance to look into this?
Flags: needinfo?(masayuki)
Unfortunately, no for now. This is still in my queue, but I have a lot of work now. So, I need more days for starting this.

Looks like this has to be fixed by me. How much important this for the blocking bug?
Flags: needinfo?(masayuki)
That bug is blocked on other things too, for the moment, so this is not the only thing blocking it.  If we get to the point where this is the only problem left before I can land the other bug, I'll comment here.
If you don't touch nsIMEStateManager nor nsTextStateManager, then, actually it must not cause any regression about the stuff tested by the test. So, even if I failed to fix it before you need, I think that you don't mind this.

But I try to find time to work on this next week or next of it.
I got a change to fix this today. Now testing on tryserver:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=420bcd14cda9
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Component: Event Handling → Widget
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Created attachment 728208 [details] [diff] [review]
Patch

The causes of the failures are:

1. If the focus is moved to a design mode editor, the IME status is changed before the previous focused content loses focus because any element doesn't take focus in design mode document, which happens before the previous focused element loses focus. Therefore, when MozIMEFocusOut event is fired at blurring the previous focused element, the IME status has already been updated.

2. If the container of the elements has contenteditable="true", the test is testing the focus move from the container to one of its children. Therefore, if the new focused element's IME enabled state isn't changed from the container's state, it's doing wrong check.

So, at #1, the IME state should be enabled since nsIMEStateManager has already been changed the IME state before dispatching the MozIMEFocusOut event.

And at #2, the not changing IME state should be checked.
Attachment #728208 - Flags: review?(bzbarsky)
Comment on attachment 728208 [details] [diff] [review]
Patch

>+            if (aFocus.toDesignModeEditor) {

aTest.toDesignModeEditor, right?

r=me with that.  Thank you!
Attachment #728208 - Flags: review?(bzbarsky) → review+
Oh yes...
https://hg.mozilla.org/mozilla-central/rev/f03b609b76f2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.