Currently, widget sends compositionupdate event and text event separately. compositionupdate event is fired before text event only when composition string is updated. This check is checked by each widget. text event is fired every composition change including only clause/caret information change. nsEditor handles only this event. I'm planning that: * WidgetCompositionEvent would have text ranges which is managed by refcounting. (This reduces run-time cost) * widget would never dispatch text event. * TextComposition would dispatch text event for nsEditor. After this bug, I'll get rid of WidgetTextEvent.

By the changes of bug 975383, TextComposition will handle only text event. And compositionupdate event is dispatched from TextComposition when a text event changes composition string. Therefore, what we should do here are: 1. WidgetCompositionEvent should have the text ranges. 2. NS_TEXT_TEXT should be replaced with NS_COMPOSITION_UPDATE_INTERNAL. 3. WidgetCompositionEvent should be used for the new message instead of WidgetTextEvent. 4. Remove WidgetTextEvent. Then, all composition changes are managed by only composition events.

First of all, for making following patches smaller and easier to read, we should make WidgetCompositionEvent.data and WidgetTextEvent.theText same name. In the modern coding style, they should be mData. Therefore, this patch renames WidgetTextEvent::theText to WidgetTextEvent::mData.

This patch renames WidgetCompositionEvent::data to WidgetCompositionEvent::mData.

WidgetTextEvent has isChar for UIEvent.isChar attribute. However, it's never set true and it's not implemented by WidgetCompositionEvent. So, we can just remove it. NOTE: UIEvent.isChar isn't defined by any web standards.

NS_TEXT_TEXT event should be renamed to NS_COMPOSITION_CHANGE because it notifies editor of any composition string change including clause information. Note that the difference between NS_COMPOSITION_UPDATE and NS_COMPOSITION_CHANGE is that NS_COMPOSITION_UPDATE is fired *only* when composition *string* is changed. On the other hand, NS_COMPOSITION_CHANGE (NS_TEXT_TEXT) is fired even when composition string is *not* changed. It may be fired for notifying editor of clause boundary change and/or caret position change. Additionally, DOM compositionupdate which is caused by NS_COMPOSITION_UPDATE should be dispatched *before* actually composition string is changed. Therefore, we need to keep dispatching two events for DOM compositionupdate event and an internal event for notifying nsEditor of a change of composition string information.

And we should call NS_COMPOSITION_CHANGE event as "compositionchange" event internally. If we need to document its DOM event, then, we should call it as "DOM text event". The "text" event might be handled by some addons, therefore, we shouldn't change the name for the compatibility but I don't like NS_COMPOSITION_TEXT for its name. Because "text" event isn't clear what the event notifies somebody of.

Before changing from WidgetTextEvent to WidgetCompositionEvent for NS_COMPOSITION_CHANGE event class, we need to duplicate all members of WidgetTextEvent to WidgetCompositionEvent.

This patch changes all WidgetTextEvent instances to WidgetCompositionEvent's. The variable names of them will be renamed later. This patch makes some code which handle both WidgetTextEvent and WidgetCompositionEvent simpler!

I wonder, document.createEvent("textevent") should keep creating DOM UIEvent rather than CompositionEvent?

Just removes WidgetTextEvent completely.

Renaming variables of WidgetTextEvent to compositionChangeEvent, compChangeEvent or something. I used similar name around the code.

Renaming method names which handled WidgetTextEvent.

Some methods have taken both WidgetCompositionEvent and WidgetTextEvent by its argument. This patch changes such method's argument from WidgetGUIEvent to WidgetCompositionEvent. This is clearer and reduces the number of virtual calls of AsCompositionEvent().

Renaming EventUtils.synthesizeText() to EventUtils.synthesizeCompositionChange().

Now, we can get rid of PBrowser.TextEvent(). Instead, ESM should use SendCompositionEvent() for NS_COMPOSITION_CHANGE.

Basically, these patches almost don't change actual behavior. The only one difference is that DOM event which is caused by NS_COMPOSITION_CHANGE is changed from UIEvent to CompositionEvent. I believe that this must not cause any problems with actual addons. And also I believe that it does makes sense that WidgetCompositionEvent always causes DOM CompositionEvent even if it's a non-standard event. (With this change, text event handler can access its data attribute.)

Comment on attachment 8499334 [details] [diff] [review] part.3 Remove WidgetTextEvent::isChar since it's always false on all platforms lovely :/ I wonder if isChar was ever really used. It is ancient code.

Comment on attachment 8499336 [details] [diff] [review] part.4 Rename NS_TEXT_TEXT to NS_COMPOSITION_CHANGE and fix comments which mention text events > } else if (aType.EqualsLiteral("compositionupdate")) { > // Now we don't support manually dispatching composition update with this >- // API. compositionupdate is dispatched when text event modifies >- // composition string automatically. For backward compatibility, this >- // shouldn't return error in this case. >+ // API. compositionupdate is dispatched when compositionchange event The event is still "text". > NON_IDL_EVENT(text, >- NS_TEXT_TEXT, >+ NS_COMPOSITION_CHANGE, This adds some inconsistency. "text" event, but NS_COMPOSITION_CHANGE message. Please add comment that "text" is a legacy event. We should add some telemetry to check if anyone in the web is actually using "text", and if not, stop exposing it there. >@@ -167,21 +167,26 @@ > #define NS_MUTATION_END (NS_MUTATION_START+6) > > #define NS_USER_DEFINED_EVENT 2000 > > // composition events > #define NS_COMPOSITION_EVENT_START 2200 > #define NS_COMPOSITION_START (NS_COMPOSITION_EVENT_START) > #define NS_COMPOSITION_END (NS_COMPOSITION_EVENT_START + 1) >+// NS_COMPOSITION_UPDATE is the message for DOM compositionupdate event. >+// This event should NOT be dispatched from widget since it's dispatched s/it's/it will be/ >+// by mozilla::TextComposition automatically if NS_COMPOSITION_CHANGE event >+// will change composition string. s/will change/changes/ >+// NS_COMPOSITION_CHANGE is the message for representing a change of >+// composition string. This should be dispatched from widget even if >+// composition string isn't changed but the ranges are changed. This causes >+// cause DOM text event which is non-standard DOM event. perhaps s/text event/"text" event/

Comment on attachment 8499339 [details] [diff] [review] part.5 Copy mRanges and related methods from WidgetTextEvent to WidgetCompositionEvent >+ aResult->mRanges = new mozilla::TextRangeArray(); >+ if (!aResult->mRanges) { 'new' is infallible by default, so no need for the null check.

Comment on attachment 8499344 [details] [diff] [review] part.7 Get rid of WidgetTextEvent So great.

Comment on attachment 8499345 [details] [diff] [review] part.8 Rename variable names of NS_COMPOSITION_CHANGE event >- WidgetCompositionEvent textEvent(true, NS_COMPOSITION_CHANGE, widget); >- textEvent.time = PR_IntervalNow(); >- textEvent.mData = mString; >+ WidgetCompositionEvent compChangeEvent(true, NS_COMPOSITION_CHANGE, widget); Perhaps this could be compositionChangeEvent or changeEvent or compositionChange > TextComposition::EditorWillHandleTextEvent( >- const WidgetCompositionEvent* aTextEvent) >+ const WidgetCompositionEvent* aCompChangeEvent) aChangeEvent or aCompositionChangeEvent? > TextEventHandlingMarker(TextComposition* aComposition, >- const WidgetCompositionEvent* aTextEvent) >+ const WidgetCompositionEvent* aCompChangeEvent) ditto >- void EditorWillHandleTextEvent(const WidgetCompositionEvent* aTextEvent); >+ void EditorWillHandleTextEvent( >+ const WidgetCompositionEvent* aCompChangeEvent); ditto > nsPlaintextEditor::UpdateIMEComposition(nsIDOMEvent* aDOMTextEvent) > { > NS_ABORT_IF_FALSE(aDOMTextEvent, "aDOMTextEvent must not be nullptr"); > >- WidgetCompositionEvent* widgetTextEvent = >+ WidgetCompositionEvent* compChangeEvent = > aDOMTextEvent->GetInternalNSEvent()->AsCompositionEvent(); and here >- WidgetCompositionEvent textEvent(true, NS_COMPOSITION_CHANGE, this); >- InitEvent(textEvent, nullptr); >- textEvent.mData = mIMEComposingText; >- DispatchEvent(&textEvent); >+ WidgetCompositionEvent compChangeEvent(true, NS_COMPOSITION_CHANGE, this); and here and elsewhere. Perhaps use compositionChange name consistency everywhere. That fixed, r+

Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/aad726477799 https://hg.mozilla.org/integration/mozilla-inbound/rev/3b622a7464cc https://hg.mozilla.org/integration/mozilla-inbound/rev/d2eebbc42474 https://hg.mozilla.org/integration/mozilla-inbound/rev/279373ac52ff https://hg.mozilla.org/integration/mozilla-inbound/rev/97aa7fef7adc https://hg.mozilla.org/integration/mozilla-inbound/rev/5216b7b1e4e3 https://hg.mozilla.org/integration/mozilla-inbound/rev/bd15674419d1 https://hg.mozilla.org/integration/mozilla-inbound/rev/93374aaa8f6e https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5abfd3505c https://hg.mozilla.org/integration/mozilla-inbound/rev/52273860a2cd https://hg.mozilla.org/integration/mozilla-inbound/rev/6a965889d97e https://hg.mozilla.org/integration/mozilla-inbound/rev/f17271ef08d3

https://hg.mozilla.org/mozilla-central/rev/aad726477799 https://hg.mozilla.org/mozilla-central/rev/3b622a7464cc https://hg.mozilla.org/mozilla-central/rev/d2eebbc42474 https://hg.mozilla.org/mozilla-central/rev/279373ac52ff https://hg.mozilla.org/mozilla-central/rev/97aa7fef7adc https://hg.mozilla.org/mozilla-central/rev/5216b7b1e4e3 https://hg.mozilla.org/mozilla-central/rev/bd15674419d1 https://hg.mozilla.org/mozilla-central/rev/93374aaa8f6e https://hg.mozilla.org/mozilla-central/rev/4f5abfd3505c https://hg.mozilla.org/mozilla-central/rev/52273860a2cd https://hg.mozilla.org/mozilla-central/rev/6a965889d97e https://hg.mozilla.org/mozilla-central/rev/f17271ef08d3

Modified for this change: https://developer.mozilla.org/en-US/docs/Mozilla/IME_handling_guide