Closed Bug 564411 Opened 10 years ago Closed 3 years ago

nsIEditorIMESupport methods/attributes should be moved to nsIEditor (or nsIPlaintextEditor)

Categories

(Core :: DOM: Editor, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [good first bug])

Attachments

(1 file)

I think that nsIEditorIMESupport should be removed because we always support IME on all editors and scriptable methods are not many.
Summary: nsIEditorIMESupport methods/attributes should be moved to nsIEditor → nsIEditorIMESupport methods/attributes should be moved to nsIEditor (or nsIPlaintextEditor)
Basically, nsIPlaintextEditor is better interface because IME is an UI for text editing. So, if there is non text editable editor inherits nsEditor, IME shouldn't be usable. However, nsIEditor is more convenient for editor users (especially script) because it's first interface they can get from DOM element or DOM window.

I think that the methods should be in nsIEditor and the implementations should be moved to nsPlaintextEditor.
Looks like that some addons use nsIEditorIMESupport. I guess that they copied some lines from our toolkit for checking if there is composition.
https://mxr.mozilla.org/addons/search?string=nsIEditorIMESupport

So, we cannot remove nsIEditorIMESupport. But for reducing redundant QI, we should still move the members of nsIEditorIMESupport. I was thinking that the moved interface should be nsIPlaintextEditor. However, if we do so, we cannot reduce the number of QI. Additionally, it's not realistic scenario that somebody implements new editor only with nsIEditor.

Therefore, I think that we can move the members of nsIEditorIMESupport to nsIEditor. Additionally, I guess that we can merge nsIEditor and nsIPlaintextEditor in the future (but not scope of this bug).
Humm, except to our addons, this interface is 1 addon only...
Also, c-c uses it into editor code.  so we should move all members to nsIEditor at first, then remove this interface per comment #1 and comment #2.
Whiteboard: [good first bug]
Comment on attachment 8820143 [details]
Bug 564411 Move all methods/attributes of nsIEditorIMESupport to nsIEditor

https://reviewboard.mozilla.org/r/99688/#review100122

::: editor/nsIEditorIMESupport.idl:9
(Diff revision 1)
> -
> -native IMEState(mozilla::widget::IMEState);
>  
>  [scriptable, uuid(0ba7f490-afb8-46dd-87fc-bc6137fbc899)]
> -
>  interface nsIEditorIMESupport : nsISupports

If we want to ensure addons and thunderbird etc which might use this interface keep working, perhaps nsIEditorIMESupport should inherit nsIEditor, not nsISupports
Attachment #8820143 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #7)
> Comment on attachment 8820143 [details]
> Bug 564411 Move all methods/attributes of nsIEditorIMESupport to nsIEditor
> 
> https://reviewboard.mozilla.org/r/99688/#review100122
> 
> ::: editor/nsIEditorIMESupport.idl:9
> (Diff revision 1)
> > -
> > -native IMEState(mozilla::widget::IMEState);
> >  
> >  [scriptable, uuid(0ba7f490-afb8-46dd-87fc-bc6137fbc899)]
> > -
> >  interface nsIEditorIMESupport : nsISupports
> 
> If we want to ensure addons and thunderbird etc which might use this
> interface keep working, perhaps nsIEditorIMESupport should inherit
> nsIEditor, not nsISupports

Oh, right. I forgot to change it, thanks!
Comment on attachment 8820143 [details]
Bug 564411 Move all methods/attributes of nsIEditorIMESupport to nsIEditor

https://reviewboard.mozilla.org/r/99688/#review100122

> If we want to ensure addons and thunderbird etc which might use this interface keep working, perhaps nsIEditorIMESupport should inherit nsIEditor, not nsISupports

Okay, I made nsIEditorIMEsupport inherit nsIEditor and EditorBase not inherit nsIEditor directly.
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/20dc8f3fa6ed
Move all methods/attributes of nsIEditorIMESupport to nsIEditor r=smaug
https://hg.mozilla.org/mozilla-central/rev/20dc8f3fa6ed
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.