Closed Bug 805357 Opened 7 years ago Closed 7 years ago

nsIMEStateManager should always call nsIWidget::OnIMEFocusChange(false) when our editor loses focus

Categories

(Core :: Internationalization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod)

Attachments

(2 files, 1 obsolete file)

Currently, if we don't create nsTextStateManager, we don't call nsIWidget::OnIMEFocusChange(false). This is not good behavior for widget.
Attached patch Patch (obsolete) — Splinter Review
Attachment #675012 - Flags: review?(bugs)
Comment on attachment 675012 [details] [diff] [review]
Patch


>+bool
>+nsIMEStateManager::IsIMEAvailableExceptOnPlugin(nsIWidget* aWidget)
>+{
>+  switch (aWidget->GetInputContext().mIMEState.mEnabled) {
>+    case widget::IMEState::ENABLED:
>+    case widget::IMEState::PASSWORD:
>+      return true;
>+    default:
>+      return false;
>+  }
>+}
I have no idea what this is about.
Could you explain and add also some comment to the code.
What "ExceptOnPlugin"?
Attachment #675012 - Flags: review?(bugs) → review-
If the enabled state is PLUGIN, IME should be available but it shouldn't be managed by us. Hmm, IsEditableIMEState() is better?
Well, if you want to check PLUGIN state, check that explicitly, and don't check the other values.
  switch (aWidget->GetInputContext().mIMEState.mEnabled) {
    case widget::IMEState::DISABLED:
    case widget::IMEState::PLUGIN:
      return false;
    default:
      return true;
  }
would be much easier to understand, IMO.
By the part.1, OnIMEFocusChange(true) never needs to return NS_SUCCESS_IME_NO_UPDATES for receiving OnIMEFocusChange(false). So, we can drop the error code.
Attachment #675490 - Flags: review?(roc)
Comment on attachment 675487 [details] [diff] [review]
part.1 nsIMEStateManager should always call nsIWidget::OnIMEFocusChange(false) when our editor loses focus


>+nsIMEStateManager::IsEditableIMEState(nsIWidget* aWidget)
>+{
>+  switch (aWidget->GetInputContext().mIMEState.mEnabled) {
>+    case widget::IMEState::ENABLED:
>+    case widget::IMEState::PASSWORD:
>+      return true;
>+    case widget::IMEState::PLUGIN:
>+    case widget::IMEState::DISABLED:
>+      return false;
>+    default:
>+      MOZ_NOT_REACHED("Define whther the IME enable state is editable or not");
'whther'
But maybe "Unknown IMEState" or something like that
Attachment #675487 - Flags: review?(bugs) → review+
You need to log in before you can comment on or make changes to this bug.