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.

Sign up for GitHub

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

Jump to bottom

What should happen when "deleteByComposition" is canceled? #42

Closed
whsieh opened this issue Oct 18, 2016 · 9 comments
Closed

What should happen when "deleteByComposition" is canceled? #42

whsieh opened this issue Oct 18, 2016 · 9 comments

Comments

@whsieh
Copy link

whsieh commented Oct 18, 2016

According to the spec, "preventing the default action for [deleteByComposition] will mean that [the recomposed] range will not be removed from the DOM by the user agent." However, does this mean that the page should still show UI for editing the composition (for instance, a list of candidates) and allow typing to edit the composition, but just not modify the DOM in the process? If so, would the input events dispatched for a page that prevents default on both "deleteByComposition" and "insertFromComposition" look like this when recomposing text:

Dispatch beforeinput event of type "deleteByComposition"
< preventDefault() is called, and we keep the text being recomposed exactly the same >

< user selects a candidate/types something >
Dispatch beforeinput event of type "insertCompositionText"
Dispatch input event of type "insertCompositionText"

< user selects a candidate/types something >
Dispatch beforeinput event of type "insertCompositionText"
Dispatch input event of type "insertCompositionText"

< ...and so on... >

< finally, user hits enter to commit the composition >
Dispatch beforeinput event of type "insertFromComposition"
Dispatch input event of type "insertFromComposition"

^ I also had a question about the behavior of insertFromComposition in this circumstance, but Ryosuke added a question covering that here: #41
@whsieh
Copy link
Author

whsieh commented Oct 18, 2016

@johanneswilm
Copy link
Contributor

johanneswilm commented Oct 18, 2016 •
edited
Loading

@whsieh AFAIK, the composition will go ahead as planned, no matter whether or not deleteByComposition is being preventDefaulted. The first step of that is a noncancelable insertCompositionText which inserts the entire composition string into the DOM (in case of recomposition).

In the case of a recomposition, this means that if the JS preventDefaults the very first deleteByComposition, and then doesn't remove what is to become the composition string from the DOM itself, the contents of the composition string will be seen twice: once as it was before, and a second time in the composition string.

I've tried to summarize the procedure in this note: https://w3c.github.io/input-events/#h-note4 . We should probably additionally have some examples.

@rniwa
Copy link

rniwa commented Oct 18, 2016

Note is non-normative, this really needs to be spec'ed in normative text somewhere.

@johanneswilm
Copy link
Contributor

Note is non-normative, this really needs to be spec'ed in normative text somewhere.

Yes, it is. I primarily added it as a help for JS developers who will try to use these events to create editors based on the beforeinput event. It is marked as non-normative, as it's not meant to define new behavior for browser implementers.

Is the note's content the same content you would like to see as normative text for implementers?

@rniwa
Copy link

rniwa commented Oct 18, 2016

The current node you linked isn't precise enough as normative text. We need to spec precisely what happens for each event but that the normative text probably needs to be in ui events instead.

@travisleithead @garykac

@johanneswilm
Copy link
Contributor

johanneswilm commented Oct 18, 2016 •
edited
Loading

We need to spec precisely what happens for each event

What do you mean by "each event" in this case? Do you mean to say you want the contents of the note referenced above, but in a more technical language? Or is it something completely different?

Gary has been starting a new spec on event order: https://github.com/garykac/event-order/blob/master/index.bs because ui events is too full.

@rniwa
Copy link

rniwa commented Oct 18, 2016

What do you mean by "each event" in this case? Do you mean to say you want the contents of the note referenced above, but in a more technical language? Or is it something completely different?

We need to specify the ordering of each event, and what happens when each one of the event is canceled, or DOM is mutated, etc...

Gary has been starting a new spec on event order: https://github.com/garykac/event-order/blob/master/index.bs because ui events is too full.

That's good. We really need to spec the precise event ordering there.

@annevk : I guess this is what you want re: processing model.

@annevk
Copy link
Member

annevk commented Oct 18, 2016

Not sure, it's not so much about order (although that's obviously a part of it), it's mostly how they influence each other, get their state, perform actions (such as drag-and-drop), etc.

@chong-z
Copy link
Contributor

chong-z commented Oct 18, 2016

In short preventing 'deleteByComposition' should behave similar to collapse selection forward and start composition (but will probably pre-insert some marked text).

@rniwa Yes I agree that we need some precise definition for each inputType. But before that happens #34 (comment) is the original proposal for IME related Default Action and Event Order, hope that helps.

@whsieh whsieh closed this as completed Oct 20, 2016
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
@whsieh
Support (insertFrom|deleteBy)Composition and (insert|delete)Compositi…
c824bf1 
…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
@Azmisov Azmisov mentioned this issue Oct 6, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@rniwa @johanneswilm @whsieh @annevk @chong-z