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.

Required fixes

• Correct updateCommands & updateCommandsContext order.
• #10483.
• Incorrect selection position after setData on inline editor (described in #10438).
• No selection position (empty elements path) in fresh editor.

[Piotrek Koszuliński]

I described possible fix for the initial selection position here - ​http://dev.ckeditor.com/ticket/10438#comment:8

The second part of this issue - startup events order needs to be precisely debugged and fixed in this ticket.

[Wim Leers]

I tried to grok this ticket.

I think what I'm about to say is rather controversial.

---

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 the fundament upon which the reasoning in this ticket is built. The ticket correctly points out bugs while starting from this assumption.

However, I wonder if it makes sense at all to have "an initial element path of the editor"? This implies there is a non-existent, invisible cursor. Why does this make sense? Shouldn't all context-sensitive buttons be disabled while a given editor instance does not have focus (and hence no actual caret)?

In other words: in my very humble opinion (and I'm probably oversimplifying by saying this) it makes more sense to have no "initial element path of the editor", which means that all context-sensitive buttons would be disabled until the editor instance gains focus (and hence gets an actual caret)?

[Olek Nowodziński]

Replying to Wim Leers:

I tried to grok this ticket.

I think what I'm about to say is rather controversial.

---

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 the fundament upon which the reasoning in this ticket is built. The ticket correctly points out bugs while starting from this assumption.

However, I wonder if it makes sense at all to have "an initial element path of the editor"? This implies there is a non-existent, invisible cursor. Why does this make sense? Shouldn't all context-sensitive buttons be disabled while a given editor instance does not have focus (and hence no actual caret)?

In other words: in my very humble opinion (and I'm probably oversimplifying by saying this) it makes more sense to have no "initial element path of the editor", which means that all context-sensitive buttons would be disabled until the editor instance gains focus (and hence gets an actual caret)?

Your point of view is understandable and reasonable. There's nothing controversial in your words. However...

CKEditor always tried to mimic desktop WYSIWYG editors (i.e. MS Word, (Open|Libre)Office, and older). Why? Because people expect things to working just like in their favorite text editor where the caret is visible (and blinking) since very beginning, determining states of various commands. This is the informal, social-driven standard of thinking about "freshly loaded" WYSIWYG editor.

Therefore, as you see, we cannot basically turn off all the features and wait until editor is manually focused to update states. If we did so, people would think: "this editor is totally disabled, WTF?". Users click toolbar before they write so they expect the toolbar to be working at this stage. This is why we need to enable it "in the context of something" and the best option is the very first focusable (editable) place which, generally speaking, is a paragraph in most cases.

Anyway, if we implemented initial command states according to what you proposed, editor would look kinda like after editor.setReadOnly().

[Piotrek Koszuliński]

+1 for Olek. I was also considering whether initially (and e.g. after switching from source mode to wysiwyg mode, although such cases could be handled differently) selection should exist or not.

I think that Olek made a good point about confusion that could be caused by editor looking as disabled/readonly. Such change would be especially dangerous for those people who already worked with CKEditor, since now blurred editor has all buttons enabled.

Another problem - user focuses editor, sets selection and then blurs editor. What's the state of selection right now? Why isn't toolbar returning to the initial (disabled) state? In that case buttons should be disabled after blur, because otherwise blurred editor would look oddly comparing to other editors on the same page which haven't been focused yet.

So that wouldn't be only a change for the initial state, but also blurred editor, editor after switching between modes, etc.

[Wim Leers]

I understand the expectations wrt desktop rich text editors. A difference is of course that there is an omnipresent caret in such applications. That's not the case here. But I understand the need to emulate that experience.

Happy to be wrong :) Sorry for the noise.

[Olek Nowodziński]

Extracted tests for initial states of indent commands from #10027 to t/10439@tests-v4.

[Valerij Ivashchanka]

I think, that after introduce startDisabled property to command definition, all context-sensitive commands provided by standard plugins must be inspected for valid value of this property.

And it would be fine, create issue for this task-issue, because it's faster and easier to make than fix current issue with really valid first-time button initialization.

My issue #14761 is a special case of this problem, which will appear every time when adding to the toolbar buttons related to context-sensitive commands.

[Valerij Ivashchanka]

Of course, then current issue would be fixed, startDisabled property would not make sense, аnd must be deleted from all plugins. You may want to create a separate task for this task-issue, dependent on current issue.