Widget plugin issue (typing Korean)

[HoonKi IM]

Browser and OS : Chrome and window 7

Hi, first of all, thanks for this nice Editor

Unfortunately, when we use this, we found some bug in editor,

Actually, our team is korean. so, when we type korean in editor (using bold mode or itelic mode ...), words insert the other way(reverse)

explain detail this situation

1. select bold mode in editor
2. typing korean word in editor (I anticipate "ㄱㄴㄷ")
3. first type word(ㄱ) go last position and second type word(ㄴ) go first position
4. when I type third word(ㄷ) in editor, this word go first position and first type word(ㄱ) still last position
5. result words is that "ㄷㄴㄱ"

We start to find reason and know something up to date.

this situation just proceed, when we use 'Widget plug-in'

If we insert extraPlugins(requires widget)in config (ex:placeholder or image2 plugin), This bug proceed.

so please check this situation.

and I attache some record image.

[HoonKi IM]

bug record

[HoonKi IM]

bug record(show This!!!!!!!)

[HoonKi IM]

Hi, 6/8 I found korean typing bug.

when I typing korean in editor (bold or italic mode)

word insert backwards.

​http://dev.ckeditor.com/ticket/13377 (You can see more detail info in this link)

so we find reason in widget plugin.

widget plugin code line 2754 (code : widgetsRepo.editor.fire( 'lockSnapshot' );)

when we remove this line, korean typing Normally!!!!

but, I worried about this activity. it probably effect other plugin....

please check this logic.

[Piotrek Koszuliński]

I can confirm that similar issue is reproducible with Hiragana.

0. Enable placeholder plugin.
1. Type few characters and accept the composition - should work ok.
2. Click bold button and try the same - only one character can be typed - composition is broken.

I also confirmed that this line causes it: ​https://github.com/ckeditor/ckeditor-dev/blob/b6ccc12e6aada32c286cfbc194c519e2f673074f/plugins/widget/plugin.js#L2663

[Piotrek Koszuliński]

After some deeper investigation I ended in the usual place – selection – ​https://github.com/ckeditor/ckeditor-dev/blob/b6ccc12e6aada32c286cfbc194c519e2f673074f/core/selection.js#L842-L900

And I think that there are two issues or two possible approaches to the problem:

1. Make commit() in the widget repo to be called less frequently. It does not make sense to execute it when user is only typing.
2. Somehow block undo manager when user is composing a character. I don't know however how it could work in practice.

I would actually like to work on both topics, but the second will have a greater and more positive impact on editor. The first will be simply a performance optimisation.

[Artur Delura]

Same story as ​previously. We modify content during composition. We shouldn't block it only in undomanager, but we should prevent from modifying content during the composition. According to ​this we can listen to composition* events and set appropriate property on editor like compositionPending so plugins might check this property and react properly.

[Piotrek Koszuliński]

That would make sense... I'm only worried what happens if the code in the selection module will be clocked during composition. We will still have the ZWS chars leaking. But perhaps it's better than broken composition...

[Artur Delura]

I pushed solution proposal in ​branch:t/13377. I don't know what you mean in your last comment @Reinmar.

[Piotrek Koszuliński]

What I meant is that your patch simply block the ZWS-char-fix while user is composing. It means that it doesn't do its job then. And the job of the code that you changed is to filter out ZWS characters when editor.getData() or editor.getSnapshot() run, because otherwise this character will end up in the data or snapshot.

I never liked this implementation, but there are reasons for it. At least partially... The getData() case can be changed IMO. We could filter out the ZWS char in the data processor, however, this means that no one can use this character in they data.

However, it's not that simple with editor.getSnapshot() because with snapshot goes bookmarks. And we would need to change this bookmakrs as well. This sounds feasible, but it means a lot of work.

[Artur Delura]

I have another solution which we mentioned once. Since now ZWS-char-removal/restore was applied on each user key press. Now it's only restored when needed i.e if there is other characted in ZWS element. Otherwise we don't need it.

Changes in ​branch:t/13377b.

[Artur Delura]

I'm not sure whether I'm going the right direction. I pushed another commit to the branch.

It's gonna be broken when we call getData continuesly

setInterval( function() {
    console.clear()
    var e = CKEDITOR.instances.editor;
    console.log( 'content', e.editable().getHtml().replace( /\u200b/g, '@' ) );
    console.log( 'data', e.getData().replace( /\u200b/g, '@' ) );
}, 2000 );

Because ZWS is removed then once. But in general that code fix this issue. Also two tests fail, but I it should ​http://tests.ckeditor.dev:1030/tests/core/selection/editor, because it checks whether ZWS is restored.

[Szymon Cofalik]

Proposed solution: ​branch:t/13377c.

I tried to find a different approach to the problem. The thing that broke composition was not inserting or removing ZWS itself but selection changes. Selection unfortunately had to be moved when ZWS was removed and then restored in beforeData(). The important note is that the ZWS was removed only for snapshot.

My idea is not to remove ZWS in beforeData() from DOM but instead mark it in non-intrusive way and remove it from snapshot string.

In my solution I check ZWS's position in innerHTML of it's parent and write that value as parent's attribute. This way it can't be confused and does not make any impact on actual contents. There is one danger, though. If the contents of ZWS parent changes after we calculate it's position, it may be invalid. My question is if such situation may happen? If so, the method would need some tweaking (see below) but I think this is may be a way to go if we don't want to make workarounds like suppressing events if composition started.

A further tweak may involve saving ZWS in a globally accessible space (like editor instance) so undo manager can read it on it's own and calculate its position after all beforeUndoImage events end. This way we achieve three things:

1. We are guaranteed that editor contents won't change because of other event listeners.
2. We get rid of events in selection code.
3. We don't have to make a mess with another data attribute.

[Piotrek Koszuliński]

We need to speed up 4.5.3, so we won't be able to include this task in it.

[Piotrek Koszuliński]

@a.nowodzinski is working on this issue (and all other ZWS/IME related things) and his work in ​branch:zws looks very promising. But still... we need more time.

[Olek Nowodziński]

A ticket with the new solution #13816.

[Marek Lewandowski]

This issue will be fixed by #13816 in the next milestone.

[Marek Lewandowski]

#13801 was marked as duplicate.

[Marek Lewandowski]

Fixed with ​git:2d078ff42a by #13816.