IE: Editor selection is intermittently lost when an item is selected from a toolbar menu

[Teresa Monahan]

To reproduce, use the attached test case:

• unzip linkmenu_testcase.zip. Copy the linkmenu folder to the _source/plugins directory and the config.js file to the root ckeditor folder.
• Open any CKEditor sample in IE with debugging enabled as this sample logs content to the console.
• Enter some text into the editor and select some of the content.
• Open the toolbar menu item and click the 'Edit Link' option. This will open the link dialog and should log the selected text to the console.

Problem: The selected text is often an empty string even though there was text selected in the editor.

This occurs intermittently so it may take a few attempts. If you do not reproduce the issue at first, change the selection in the editor and try opening the link dialog from the toolbar menu again.

Note: If you use the link icon directly from the toolbar instead of the menu item, the selected text is always correct.

[Garry Yao]

Open dialog from menu item will result in a wrong selection lock in IE in some situation, we're not likely to provide a solution to the current issue while this bug is already addressed in v4.

[Jakub Ś]

A workaround that always worked for me in IE is adding editor.focus(); before getting the selection

var editor = this.getParentEditor();
editor.focus();
var selection = editor.getSelection();   
console.log('Selection is: ', selection);
console.log('Selected Text is: ', selection.getSelectedText());

This is a won't fix in v3 but should be handled properly in v4

[Teresa Monahan]

Proposed fix

[Teresa Monahan]

The suggested workaround of focusing the editor before getting the selection does not work for us. If this issue is due to a wrong selection lock in IE as per comment 1, would the attached patch be a viable solution for this? It just removes the IE specific code in the onShow() of the menu plugin which locks the selection. Why is this code in place? Would there be any undesired consequences of this change?

This issue is something our users will encounter on a very regular basis so we would really appreciate any help you can give us.

[Jakub Ś]

I'm not sure why editor.focus() didn't work in your case - could you give me a comment here. I remember testing it with default CKEditor in IE and it worked. I have tried to test this once but it seems that you have removed the plugin from the bug (not sure why).

Anyway @garry.yao has provided some patch that might work (I wasn't tested) for your case.

[Teresa Monahan]

Replying to j.swiderski:

I'm not sure why editor.focus() didn't work in your case - could you give me a comment here. I remember testing it with default CKEditor in IE and it worked. I have tried to test this once but it seems that you have removed the plugin from the bug (not sure why).

Anyway @garry.yao has provided some patch that might work (I wasn't tested) for your case.

I didn't remove the sample plugin from the testcase but I see that it is missing. I have just re-attached it.

When I added editor.focus() directly before the getSelection() call as you suggested, there was no difference. The selection was still lost intermittently.

The patch that @garry.yao provided does seem to be working well though. We will continue to test it further to make sure it doesn't have any adverse effects.

Thanks to you both for your help with this.

[Teresa Monahan]

We have found some regressions in IE with 8978_2.patch​ applied e.g.

• JavaScript error thrown when we try to edit Table Properties
• JavaScript error when we delete a table cell/row/column or the entire table
• It is not possible to change list style or start value of lists after initial style or start value is set

Therefore we need to revert this patch and find an alternative fix for the issue reported in this ticket.

Can you provide comments on 8978.patch​ which I previously suggested as a fix for this issue (details are included in comment:3 above)? Can you think of any adverse effects of this change? Do you have any other suggestions for a fix for this issue?

As mentioned previously, this is a very common usecase in our products so we really need your help with this.

[Teresa Monahan]

Replying to j.swiderski:

I'm not sure why editor.focus() didn't work in your case - could you give me a comment here. I remember testing it with default CKEditor in IE and it worked. I have tried to test this once but it seems that you have removed the plugin from the bug (not sure why).

Anyway @garry.yao has provided some patch that might work (I wasn't tested) for your case.

Hi, the patch proposed by @garry.yao introduced regressions as discussed in comment:6. Can you please provide feedback on my original proposed fix (8978.patch)?

[Garry Yao]

Thanks for the review, would u check the 8978_3.patch is enough to eliminate this issue?

[Teresa Monahan]

Replying to garry.yao:

Thanks for the review, would u check the 8978_3.patch is enough to eliminate this issue?

I tried 8978_3.patch but the original issue reported in this ticket still occurs. I also tried applying 8978_2.patch and 8978_3.patch together but this still results in the regressions mentioned in comment:6.

[Garry Yao]

8978_3.patch is confirmed to not work, 8978_4.patch is supposed to fix the issue, review again?

[Teresa Monahan]

8978_4.patch seems to address the issue in this ticket. Can you please test this to verify that it does not introduce any regressions. Thanks.

[Garry Yao]

Providing a simpler way to reproduce for the reviewer:

1. Open replacebyclass sample
2. Select the word "are"
3. Click to open "SCAYT" menu button
4. Without clicking on any menu item, close it with ESC key

• Actual: Cursor collapsed to the start of the selected text;
• Expected: Text selection remains.

There's no easy way to come out a automatic test for it though, since this bug relies on mouse/keyboard event.

[Garry Yao]

Fixed with [7518].

[Garry Yao]

Re-fixed with [7560], check #9116 for more details.