Two-step caret movement between different attributes is inconsistent

[tomalec]

this is an issue found during the development of #6722

📝 Provide a description of the improvement

I think the current behavior in regards of moving between different attributes is counter-intuitive.
1/ moving right from <i>foo[]</i><b>bar</b> moves to the <i>foo</i><b>[]bar</b>

ckeditor5/packages/ckeditor5-typing/tests/twostepcaretmovement.js

Lines 106 to 117 in 48f5c95

	it( 'should use two-steps movement when between nodes with the same attribute but different value', () => {
	setData( model, '<$text a="1">bar[]</$text><$text a="2">foo</$text>' );
	testTwoStepCaretMovement( [
	{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 },
	'→',
	// <$text a="1">bar</$text>[]<$text a="2">foo</$text>
	{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 },
	'→',
	// <$text a="1">bar</$text><$text a="2">[]foo</$text>
	{ selectionAttributes: [ 'a' ], isGravityOverridden: true, preventDefault: 2, evtStopCalled: 2 },
	'→',

(BTW, the tests description does not match the test code which actually tests "three-steps movement when between nodes with the same attribute but different value")
2. moving right from <a href="f">foo[]</a><a href="b">bar</a> moves to the <a href="f">foo</a>[]<a href="b">bar</a>

ckeditor5/packages/ckeditor5-typing/tests/twostepcaretmovement.js

Lines 77 to 90 in 48f5c95

	it( 'should "enter" the text with attribute in two steps', () => {
	setData( model, '<$text c="true">foo[]</$text><$text a="true" b="true">bar</$text>' );
	testTwoStepCaretMovement( [
	// Gravity is not overridden, caret is at the beginning of the text but is "outside" of the text.
	{ selectionAttributes: [ 'c' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 },
	'→',
	// Gravity is overridden, caret movement is blocked, selection at the beginning but "inside" the text.
	{ selectionAttributes: [ 'a', 'b' ], isGravityOverridden: true, preventDefault: 1, evtStopCalled: 1 },
	'→',
	// Caret movement was not blocked this time (still once) so everything works normally.
	{ selectionAttributes: [ 'a', 'b' ], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 }
	] );
	} );

As a user, I find this inconsistency confusing.

You are able to type in between the same attribute but different values, but not between two different attributes.
If it would be inconsistent, I 'd rather make it the other way around. If it's the same attr, I'd just move between attribute values, not insert a character without any value. Then, as switching between entire attrs drops the first attribute anyway, I could expect writing attr-free here.

Needless to mention, that such inconsistency requires way more complicated code.

So far there was no other attribute than linkHref that used 2SCM (other than artificial ones made up for tests).

Solution

I propose to change the behavior of 2SCM for a link to match the test description, NOT the test code, and make it really two step.
It would break the currently exposed link behavior in favor of not used so far multi-attribute behavior. However, I believe, it would be more intuitive when more attributes or cases comes to play:

Consider 2SCM is enabled for: link, bold, code, but is not enabled for: ital

<$t link code>foo[]</$t><$t bold code>bar</$t>	+ → =	<$t link code>foo</$t><$t bold code>[]bar</$t>
<$t link>foo[]</$t><$t bold>bar</$t>	+ → =	<$t link>foo</$t><$t bold>[]bar</$t>
<$t link=f code>foo[]</$t><$t link=b code>bar</$t>	+ → =	<$t link=f code>foo</$t><$t link=b code>[]bar</$t>
<$t link=f ital>foo[]</$t><$t link=b ital>bar</$t>	+ → =	<$t link=f ital>foo</$t><$t link=b ital>[]bar</$t>

The explanation of the above behavior is simple:

When at least one of the 2SCM-engaged-attributes changes its value (incl. starts, ends) at a given position, the plugin creates two artificial positions for the caret. Each position, allows the user to type using the set of attributes used on its side.

Putting <$t link=f>foo[]</$t><$t link=b>bar</$t> + → = <$t link=f >foo</$t><$t>[]</$t><$t link=b>bar</$t>, to the picture above, would require more explanations and expectations for cases like
<$t link=f code ital>foo[]</$t><$t link=b code ital>bar</$t> + → = ?

📃 Other details

• Browser: all
• OS: all
• CKEditor version: master
• Installed CKEditor plugins: basic-styles, typing, link

Related issues:

• Rethink two-step caret movement #4341
• Two-step caret movement fails at the end of paragraph - Firefox only #7528
• Refactor the way 2SCM support multiple attributes. Enable it for code. #7577

---

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

[Reinmar]

If our goal was to allow putting the caret in any of the possible locations in the text to type with any of the attributes, we should indeed have not two-step caret movement, but multi-step caret movement :D Also, this isn't actually possible with just arrow keys as, when you're leaving two attributes, should you first leave A or B? If A, then how to type with A only. 

Anyway, I'd like to set one thing – our goal is not to handle any possible situation but to improve the UX in common cases.

Let's consider something like:

The <a href="#"><code>Editor.create()</code></a> method

As a user, the most probable things that I'd like to do are:

• keep typing in <a+code> if I'm still typing "Editor.create()",
• type outside to add " method...",
• type " method" inside <a>, but outside code if I like " method" to be linked too,

The 1st is obvious. The 2nd would be solved by two-step caret movement (with one step to leave both). The 3rd can be achieved by clicking the Code button or is obvious if you actually consider how such content is usually created:

1. first type: "The Editor.create() method"
2. then link the "Editor.create() method" part

What do I miss? Oh, yeah – cases that @tomalec mentions in the ticket :D 

The thing is – I can't see real-life common scenarios in which you'd have this:

<a hred="#foo">Foo</a><a href="#bar">Bar</a>

That's because there's usually a space between both links (or any other formatting, as we usually format entire words) or the content was created in steps like:

1. type "Foo Bar"
2. link "Foo"
3. link "Bar"

That's why the current three-step caret movement seems unnecessary to me. And since any multi-step caret movement seems to be a bit confusing to users, I'm for simplifying its behavior to the absolute minimum, especially if that simplifies the implementation too.

[Reinmar]

Quoting @tomalec:

The change would be a simple byproduct, but of a big rewrite, so I'm not sure if it would be easy to review.

So, it's a question whether you're rewriting this anyway to add support to running this on two attributes. If the review will be tough anyway it does not matter that much if we add yet another case. I tend to look at the final product anyway, not particular changes. However, at some point we have to limit the scope of the original ticket to simply finish it one day.

[tomalec]

I read the above as:

1. The ideal solution would be to allow user "escape" from desired attribute(s) at any moment - that's not possible with current engine (a+code=code+a=don't know which one to escape first), or would introduce enormous UX problem.
2. Let's focus on major use-cases - support basic use cases in intuitive manner, leave the other to be the easiest to implement.

TL;DR
Do you agree to, make it "maximum two steps for single caret position" as not perfect, but easiest to exaplin to users and implement?

[tomalec]

So, it's a question whether you're rewriting this anyway to add support to running this on two attributes.

I'm rewriting it to make 2 step behave intuitive to me in the cases I consider basic, and to make it 2-steps maximum when leaving <$text a b>foo[]</$text>+→ - what I believe was the reason to reject #6722. The PR that solved the original user request to have something for the most basic real-world use case: <$text linkHref="g.com">foo</$text> bar <$text code>baz[]</$text>+→`.

---

If the review will be tough anyway it does not matter that much if we add yet another case.

but isn't this case YAGNI? If so why add it, not just for implementation and review effort, but the future maintenance cost as well.

[Reinmar]

Do you agree to, make it "maximum two steps for single caret position" as not perfect, but easiest to exaplin to users and implement?

Yes.

[tomalec]

... our goal is not to handle any possible situation but to improve the UX in common cases.

That's why I proposed to optimize for something I consider common cases.

• <$t link>foo[]</$t><$t>bar</$t><$t bold>baz</$t>
• <$t link>foo[]</$t><$t bold>baz</$t>
• <$t link bold>foo[]</$t>
• <$t bold>foo</$t><$t link bold>bar[]</$t><$t bold>baz</$t>
• <$t link ital>foo[]</$t><$t ital>bar</$t>

If you disagree, please provide the alternative list of common cases :)

[Reinmar]

> If the review will be tough anyway it does not matter that much if we add yet another case.

but isn't this case YAGNI? If so why add it, not just for implementation and review effort, but the future maintenance cost as well.

By adding "yet another case" I meant "changing yet another thing". Since, you're actually working on something else (getting rid of 3-step movement wasn't in the original story) it theoretically should not be included in the PR. However, if you're currently rewriting this feature anyway, it makes no sense to try to maintain the 3-step behavior if it's problematic.

Is my point clearer now?

[tomalec]

Is my point clearer now?

Yes, thanks :)

[Reinmar]

That's why I proposed to optimize for something I consider common cases.

I'd say half of them are not common IRL. You rarely have one attribute adjacent to another attribute (`<i>x</i><b>y</b>`). There's usually a text node between them (`<i>x</i> <b>y</b>`).

Anyway, I asked initially about more examples, not test cases. Examples helped me understand on conceptual level what you want to change. The list that you generated looks more like tests – proving the stability of the algorithm.

For me, it's enough here to understand that we agree regarding:

Do you agree to, make it "maximum two steps for single caret position" as not perfect, but easiest to exaplin to users and implement?

And that you want to ensure maximum two steps by changing gravity for all attributes at once:

<$t link code>foo[]</$t><$t bold code>bar</$t>

+ → =

<$t link code>foo</$t><$t bold code>[]bar</$t>

Which I agree with as this is the most obvious solution.