Closed
Bug 840837
Opened 12 years ago
Closed 12 years ago
test_imestate testing for MozIMEFocusOut is broken
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: bzbarsky, Assigned: masayuki)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
Masayuki, could you look at this?
Assignee | ||
Comment 2•12 years ago
|
||
Yep, probably, I will have some time for this next week.
Reporter | ||
Comment 3•12 years ago
|
||
Masayuki-san, have you had a chance to look into this?
Flags: needinfo?(masayuki)
Assignee | ||
Comment 4•12 years ago
|
||
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)
Reporter | ||
Comment 5•12 years ago
|
||
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.
Assignee | ||
Comment 6•12 years ago
|
||
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.
Assignee | ||
Comment 7•12 years ago
|
||
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
Assignee | ||
Updated•12 years ago
|
Component: Event Handling → Widget
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Assignee | ||
Comment 8•12 years ago
|
||
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)
Reporter | ||
Comment 9•12 years ago
|
||
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+
Assignee | ||
Comment 10•12 years ago
|
||
Oh yes...
Assignee | ||
Comment 11•12 years ago
|
||
Comment 12•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•