Closed Bug 962920 Opened 6 years ago Closed 6 years ago

[TSF] Should commit composition at entering modal state

Categories

(Core :: Widget: Win32, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached file testcase (obsolete) —
Even if modal dialog such as file picker is opened during composition, composition isn't committed.

Especially in TSF mode, this makes TIP confused. We should add new notification NOTIFY_IME_OF_ENTERING_MODAL_STATE and nsTextStore should handle it.

STR:

0. Allow popup on the test page
1. Open the testcase
2. Start composition, then, file picker is opened per twice compositionupdate.
Attached file testcase
Hmm, handling WM_ENTERIDLE has a problem. The composition events for committing composition are fired *while* in modal state. This means that web contents can run in the modal loop.
Attachment #8364108 - Attachment is obsolete: true
Ah, but looks like it's another bug. Even while a file picker is open, setInterval() works.
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
Handling WM_ENTERIDLE message in nsTextStore is enough at least for now.

While a native modal dialog such as file picker, color picker or print dialog is open, the parent window receives almost only WM_ENTERIDLE. So, if WM_ENTERIDLE is received on a window which has composition, nsTextStore should commit the composition. It means that we ungrab IME from nsTextStore. Then, users will be able to use IME on the modal dialog.

By this change, compositionupdate, compositionend and somebody handlers can run while native modal dialog is open. But this doesn't break Gecko's design because setInterval() is also available. So, it's not the first way to run script under native modal dialog.

In IMM mode, we have same issue. However, even if we request IME to commit the composition, WM_IME_COMPOSITION* messages will be handled by the native modal dialog since focus is already moved and active IMM context is the dialog's. So, we cannot fix this bug in IMM mode easy.

As I commented in comment #0, we should use nsIWidget::NotifyIME() when we call API to open native modal dialog. However, there is no good point to insert the call. We need to add it for each opener. Therefore, it's not so better than this patch. And when we will support new native dialog, we need to add it. But probably adding a call of nsIWidget::NotifyIME() must be forgotten. So, this approach must not be easy to maintain.

In conclusion, we should fix this bug only in TSF mode and handle WM_ENTERIDLE directly. This is the simplest fix.
Assignee: nobody → masayuki
Attachment #8453748 - Flags: review?(jmathies)
Comment on attachment 8453748 [details] [diff] [review]
Patch

Review of attachment 8453748 [details] [diff] [review]:
-----------------------------------------------------------------

sgtm!
Attachment #8453748 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/mozilla-central/rev/7d98395828c7
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.