Initial command states are naive, biased and buggy

[Olek Nowodziński]

The problem emerged while testing #10027.

Symptoms

We expect command states to be properly set when editor is ready. A state of every context-sensitive command should reflect an initial element path of the editor, the same way command states are updated when selection start changes and element path is updated.

This is, however, working not quite as good as expected. Some commands (and UI buttons) have wrong states when editor starts. This suggests that some actions are unavailable when, apparently, they can be used immediately before clicking in editable and displaying the caret. Of course, the issue has alternatives: some features are TRISTATE_OFF instead of TRISTATE_ON, TRISTATE_OFF instead of TRISTATE_DISABLED etc.

We missed this bug because in most cases "wrong" command states match the initial content of the editor and it's hard to see this as there are so many buttons to be checked. Also we introduced startDisabled property to command definition (i.e. unlink command) to force initial TRISTATE_DISABLED. This creates a delusion of the correct initial state although it takes the advantage of the fact that we hardly ever put <a> tag at the beginning of the content (unlink case).

Examples

Open attached commandstates.html​ in Chrome (the last chapter explains why). Use t/10027 branch as the code basis because this sample uses indentlist plugin.

The first two samples show naive unlink command behavior. Play with ​startDisabled property to see awkward situations.

The third sample should be working just fine, at least in Chrome. The fourth sample explains the origin of this ticket.

Note: when you click the very first editable element in editable (link, paragraph, list element), states are automatically corrected.

Code

So far, the origin of this issue is expected to be within updateCommands() (called on mode and readonly) and updateCommandsContext() (called on selectionChange) ​methods in core/editor.js.

The early debugging (Chrome only) revealed that updateCommandsContext() is called before updateCommands() and it sets corrects states which correspond with the selection. When updateCommands() is executed, correct states get distorted.

This can be "fixed" by changing ​a single line of code to something like this:

this.on( 'mode', function() {
	updateCommands.call( this );
	this.forceNextSelectionCheck();
	this.selectionChange( 1 );
} );

This piece of code is nothing like a gentle solution though. It's ugly and dangerous since it forces updateCommandsContext() once mode is fired. Nevertheless, it brings commandstates.html​ examples back to life by setting correct initial states.

FF and IE "thing"

The dirty fix doesn't work in FF and IE (but it could).

Please note that justify sample doesn't work in FF and IE at all (all justify commands are TRISTATE_OFF). This is because the initial elementPath in these browsers ends in body. See:

CKEDITOR.instances.editor1.elementPath().lastElement

Unfortunately, if we want to fix initial command states, the initial elementPath must be working in FF and IE just like in Chrome. This may be either very simple of very hard to do since the code responsible for initial selection is diffused (referring to talk with Reinmar).

Further testing

I'd love to see all commands tested with different initial contents of the editor. To save time, we could choose 10 or 15 critical and very common commands (buttons) and create tests to assert initial states.

This would protect us from further troubles both in inline and framed instances. We deal with this kind of bugs very often: in almost every single release some command states are fixed. Even if there are tests that check initial states of some commands, they're in different files, hard to develop and control. This approach is weak and we lose time because of this.