Error thrown in focusManager test

[Piotrek Koszuliński]

​http://ckeditor4.t/dt/core/focusManager/focus.html

Uncaught TypeError: Cannot call method 'getType' of null

First bad commit (​git:ddf9074ad).

ddf9074ad04d18d5ba6a8dcc1a9162936e455bc2 is the first bad commit
commit ddf9074ad04d18d5ba6a8dcc1a9162936e455bc2
Author: Piotrek Reinmar Koszuliński <pkoszulinski@gmail.com>
Date:   Mon May 13 11:56:42 2013 +0200

    Return null from getSelection when not in wysiwyg mode.

[Piotr Jasiun]

Changes in ​t/11153.

[Piotrek Koszuliński]

I've got strong doubts if that test is good. No one should lock selection when editor is not in a correct mode, and I see that this test tries to mock editor incorrectly, without setting range. Or executes a similar, incorrect scenario.

Please investigate it from the other side. Editor should not fail quietly if it's used incorrectly, and it seems that it is.

[Piotr Jasiun]

Replying to Reinmar:

I've got strong doubts if that test is good. No one should lock selection when editor is not in a correct mode, and I see that this test tries to mock editor incorrectly, without setting range. Or executes a similar, incorrect scenario.

Please investigate it from the other side. Editor should not fail quietly if it's used incorrectly, and it seems that it is.

It was my first idea (to investigate test workflow and make a fix in test, not in dev project).

But then I realized that lockSelection works incorrectly. lockSelection returns false if locking the selection was not possible because of wrong type of selection, what is good. But I think that it should also return false if there is no selection (i.e. because editor is in wrong editor mode) instead of throwing error.

With this fix test do not show any error. Because there is a mock instead of real editor there is no mode nor editable so lockSelection will not work, but this is not what this test (focusManager) should check.

[Piotrek Koszuliński]

1. That test creates an editor without editable and plays with focus. This is completely unrealistic case and we found it because lockSelection fails. Making it silently fail in this case would hide that issue and eventually more of them in the future.

2. There's a difference between no selection (which happens when there's no editable or we're not in wysiwyg mode) and selection of a type none.

3. We should first fix the test and then see (e.g. based on some other real life scenarios or if corrected test will still fail) if we need that condition in lockSelection.

[Piotr Jasiun]

There was missing clean up after first test. I've added 'editor.delete()' and now everything is fine. You can check test branch t/11153 with dev master.

[Piotrek Koszuliński]

Fixed on master with cebb120 on tests. Those tests are poor, but it's not a time to rewrite them (#11254).