Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What should happen when insertFromComposition is canceled #41

Open
rniwa opened this issue Oct 18, 2016 · 5 comments
Open

What should happen when insertFromComposition is canceled #41

rniwa opened this issue Oct 18, 2016 · 5 comments

Comments

@rniwa
Copy link

rniwa commented Oct 18, 2016

When insertFromComposition is canceled, we can't continue to have the text being composited since UA doesn't have the capability to keep input methods and keyboards open in some platforms. i.e. we need to remove the currently composting text from DOM.

There are two behaviors we can implement:

Fire another insertCompositionText to replace the composition with an empty string when insertFromComposition is canceled.
Before firing insertFromComposition, fire another insertFromComposition to replace the composition text with an empty string just like we do in deleteByComposition before starting composition.
(1) is weird because it would mean that we would fire an extra composition event after the composition had logically ended.

(2) makes sense because we already delete the existing content with deleteByComposition before entering input methods in reconversion case. We're simply following the same pattern when we're exiting the composition by first emptying the composing text, and then inserting the confirmed text.

@rniwa
Copy link
Author

rniwa commented Oct 18, 2016

@choniong @whsieh @ojanvafai

@johanneswilm
Copy link
Contributor

johanneswilm commented Oct 18, 2016

@rniwa Isn't the current setup the same as your option 2 as the entire composition string is deleted with a non-cancelable "deleteCompositionText" before the "insertFromComposition"?

Also, see this note: https://w3c.github.io/input-events/#h-note4

@chong-z chong-z closed this as completed Oct 18, 2016
@chong-z chong-z reopened this Oct 18, 2016
@chong-z
Copy link
Contributor

chong-z commented Oct 18, 2016

(Wrong button sorry...)

Same as what @johanneswilm said, but the assumption was UA always do full text replacing.

In fact we probably want to loose the restriction as UA might want to optimize the replacing algorithm (e.g. To preserve style.) when 'insertFromComposition' was not cancelled. Will file a separate issue for this.

@whsieh
Copy link

whsieh commented Oct 18, 2016

Thank you for your responses, @johanneswilm and @choniong! To reiterate, when the user commits the composed text to the DOM, we:

  1. Dispatch a non-cancelable beforeinput event of type "deleteCompositionText"
  2. Remove the composition text
  3. Dispatch an input event of type "deleteCompositionText"
  4. Dispatch a cancelable beforeinput event of type "insertFromComposition"
  5. If (4) was not prevented, then reinsert the final composition text into the DOM and dispatch an input event of type "insertFromComposition"

Does this sound right?

@johanneswilm
Copy link
Contributor

@whsieh Yes, that sounds right.

ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
…onText inputTypes for InputEvents

https://bugs.webkit.org/show_bug.cgi?id=163460
<rdar://problem/28784142>

Reviewed by Darin Adler.

Source/WebCore:

Adds basic support for the composition inputTypes in the InputEvent spec. See w3.org/TR/input-events,
github.com/w3c/input-events/issues/41 and github.com/w3c/input-events/issues/42 for more details. While input
events are fired in the correct order with respect to each other, additional work will be required to ensure
that input events are fired in the correct order with respect to composition(start|update|end) events and
textInput events. This is held off until the expected ordering of events is officially defined in the spec.

Tests: fast/events/before-input-events-prevent-insert-composition.html
       fast/events/before-input-events-prevent-recomposition.html
       fast/events/input-events-ime-composition.html
       fast/events/input-events-ime-recomposition.html

* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::apply):
* editing/CompositeEditCommand.h:
(WebCore::CompositeEditCommand::isBeforeInputEventCancelable):

Adds a new virtual method hook for subclasses to mark their `beforeinput` events as non-cancelable (see
TypingCommand::isBeforeInputEventCancelable). By default, `beforeinput` events are cancelable.

* editing/EditAction.h:

Adds 4 new EditActions corresponding to the 4 composition-related inputTypes. These are:
EditActionTypingDeletePendingComposition    => "deleteCompositionText"
EditActionTypingDeleteFinalComposition      => "deleteByComposition"
EditActionTypingInsertPendingComposition    => "insertCompositionText"
EditActionTypingInsertFinalComposition      => "insertFromComposition"

* editing/EditCommand.cpp:
(WebCore::inputTypeNameForEditingAction):
* editing/Editor.cpp:
(WebCore::dispatchBeforeInputEvent):
(WebCore::dispatchBeforeInputEvents):
(WebCore::Editor::willApplyEditing):
(WebCore::Editor::insertTextWithoutSendingTextEvent):
(WebCore::Editor::setComposition):

In setComposition(text, mode), tweak the logic for committing a composition to always delete the selection
before inserting the final composition text. In setComposition(text, underlines, start, end), catch the case
where we're beginning to recompose an existing range in the DOM and delete the recomposed text first.

* editing/TypingCommand.cpp:
(WebCore::editActionForTypingCommand):
(WebCore::TypingCommand::TypingCommand):
(WebCore::TypingCommand::deleteSelection):

Adds a TextCompositionType parameter so that call sites (see Editor::setComposition) can indicate what state the
edited composition is in. This allows us to differentiate between deletion of finalized composition text in
preparation of recomposing a range in the DOM, and deletion of composition text that has not yet been committed
in preparation for inserting a finalized composition into the DOM.

(WebCore::TypingCommand::deleteKeyPressed):
(WebCore::TypingCommand::forwardDeleteKeyPressed):
(WebCore::TypingCommand::insertText):
(WebCore::TypingCommand::insertLineBreak):
(WebCore::TypingCommand::insertParagraphSeparatorInQuotedContent):
(WebCore::TypingCommand::insertParagraphSeparator):
(WebCore::TypingCommand::isBeforeInputEventCancelable):
(WebCore::TypingCommand::inputEventData):
(WebCore::TypingCommand::willAddTypingToOpenCommand):
* editing/TypingCommand.h:

Source/WebKit/mac:

Handle new EditAction types for inserting/deleting pending/final compositions.

* WebCoreSupport/WebEditorClient.mm:
(undoNameForEditAction):

Source/WebKit2:

Handle new EditAction types for inserting/deleting pending/final compositions.

* UIProcess/WebEditCommandProxy.cpp:
(WebKit::WebEditCommandProxy::nameForEditAction):

LayoutTests:

Adds 4 new layout tests to verify that composition events are dispatched as expected when using IME, and that
input events of type "insertFromComposition" and "deleteByComposition" can be prevented.

Also rebaselines an existing WK1 editing test (text-input-controller.html) to account for how we now delete the
existing composition text before inserting the finalized composition text in Editor::setComposition. This means
that there are a few more delegate calls than there were before (as seen in the expected output), although the
resulting behavior is still the same.

* editing/mac/input/text-input-controller-expected.txt:
* fast/events/before-input-events-prevent-insert-composition.html: Added.
* fast/events/before-input-events-prevent-recomposition.html: Added.
* fast/events/input-events-ime-composition.html: Added.
* fast/events/input-events-ime-recomposition.html: Added.
* platform/ios-simulator/TestExpectations:


Canonical link: https://commits.webkit.org/181577@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207698 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants