IE: Clicking on element's path disables cut/copy icons.

[Jakub Ś]

1. Clear editor contents.
2. Type something inside editor.
3. Click on body in element's path.

Result: Text is selected but Cut and Copy icons are disabled.

IE is the only browser where we can make those buttons work, so it is not a low priority issue IMO.

If you empty editor, type and then click on body in element's path, buttons get disabled from CKEditor 4.0.
If however you empty editor, type, select some text and then click on body in element's path, buttons get disabled from CKEditor 4.0.2.

[Tade0]

The problem here was that in IE the sequence of events when clicking on one of the buttons in the element path was:

1. Editable loses focus.
2. Selection is saved.
3. Saved selection is used to determine whether the buttons should be active.

The problem here of course was that in IE the wrong, outdated selection was fed into the selectionChange event handler.

I solved this by explicitly firing the selectionChange event with the new selection.

Changes pushed to ​branch:t/13548.

[Marek Lewandowski]

@t.jakut I'll ask you to review this one.

[Tomasz Jakut]

The fix seems to be working, however there are some problems with it:

1. There are no tests. At least a manual test should be provided for such ticket. I'd also prepare some unit test if it won't be too tricky.
2. Because of the fix, now selectionChange event is fired twice when clicking on elementspath. I'd check if it doesn't cause any trouble.

Despite these two things, fix looks good to me.

I've rebased the branch and pushed two, tiny codestyle fixes.

[Tade0]

Added a test here: ​http://tests.ckeditor.dev:1030/tests/core/command/command#tests%2Fcore%2Fcommand%2Fcommand%20test%20copy%20command%20not%20disabled%20after%20clicking%20on%20elements%20path and rebased.

Changes pushed to ​branch:t/13548.

As for firing selectionChange twice: I checked for trouble, found nothing so far. I think that given that there are so many selectionChange listeners and all of them have side effects if there were any trouble with firing this event twice, we would know about it by now.

Alternatively the commands could be refreshed explicitly.

[Tomasz Jakut]

The test is failing in IE11:

TypeError: Unable to get property 'checkReadOnly' of undefined or null reference

Additionally it only check "Copy" command and "Cut" one is not tested. It would be also nice to have a manual test.

I've rebased the ​branch:t/13548 to the newest master.

[Marek Lewandowski]

Moving tickets to the next milestone.

[Tade0]

There seems to be a deeper problem here, namely selection.selectElement( element ) results with a selection without ranges. Investigating.

[Tade0]

The problem turned out to be embarrassingly obvious: It was impossible to create a ranged selection over a fragment that did not contain text nodes (<p><strong></strong></p>).

Fixed the test, added manual test, rebased with master.

Changes pushed to ​branch:t/13548.

[Tomasz Jakut]

There is still no unit test for cut command. Except that fact, everything seems to be working fine.

[kkrzton]

I was wondering while this fix is only for IE/Edge browsers, wouldn't it be better to wrap the ​explicit `selectionChange` event into if so it is only fired for IE/Edge browsers?

[Tade0]

Replying to k.krzton:

I was wondering while this fix is only for IE/Edge browsers, wouldn't it be better to wrap the ​explicit `selectionChange` event into if so it is only fired for IE/Edge browsers?

Perhaps, but then again selectionChange is fired a lot, sometimes with effectively the same selection, so IMO it doesn't make much of a difference.

Added cut test, rebased with master, changes pushed to ​branch:t/13548.

[Tomasz Jakut]

Actually good catch, @k.krzton!

@Tade0, @k.krzton is right – if the fix is only for IE, then it would be better if the event firing is wrapped in simple if. The same goes with unit tests in tests/core/command/command.js. And that should be really the last thing here.

[Tade0]

Changes pushed to ​branch:t/13548.

[Tomasz Jakut]

LGFM. Fixed with ​git:24f2f3c.