i/4858: Automatically add a link protocol when not provided in the link form.

[panr]

Suggested merge commit message (convention)

Feature (link): Introduced the config.link.defaultProtocol option for automatically adding it to the links when it's not provided by the user in the link form. Closes #4858.

MINOR BREAKING CHANGES (link): This PR introduces config.link.defaultProtocol API.

[panr]

@jodator As we've spoken, I went with the UI level of change... but when I was writing tests I thought that maybe we should add this feature for the model level as well?
Now LinkCommand.execute('ckeditor.com') won't add any protocol (as before), so maybe we should be more consistent here 🤔

[panr]

I still have to update feature docs, but the code can be reviewed.

[jodator]

The code generally looks OK so I'll wait for the tests. One thing to change which slipped us:

2. Should be a opt-in feature.

As for the command auto-fix. I'm not 100% sure whether it is a good or bad idea. Could you check in what scenarios the link command is used? My thinking is that if someone (dev) use the command to insert link then we shouldn't alter it but if the link command is executed by user action we should alter it. But I can't think of other options than UI.

The second scenario is handling link in pasted content. But that seems to be out-of scope for this issue.

[panr]

As for the command auto-fix. I'm not 100% sure whether it is a good or bad idea.

Me neither...

Could you check in what scenarios the link command is used? My thinking is that if someone (dev) use the command to insert link then we shouldn't alter it but if the link command is executed by user action we should alter it. But I can't think of other options than UI.

What I've seen that we're using the command only on submitting the value (but here the command already gets auto-fixed value from the form input) and in the unit tests... I haven't spotted anything else...

And maybe you're right saying that from the developer's perspective such autofixer isn't a good idea, because it can be unpredictable... So maybe we should stick with the user's perspective only.

[jodator]

So maybe we should stick with the user's perspective only.

Sounds good - let's keep that on UI part only then.

[jodator]

The code loooks good 👍 the things to address

1. RegExp objects should be created once and then re-used.
2. The configuration is static so the code should not anticipate that (Dynamic toolbar configuration options #7383)

[jodator]

👌 LGTM

[jodator]

@Reinmar Quick question about changelog and annotating breaking changes.

I've removed the MINOR BREAKING CHANGE as this is not a breaking change - it is a MINOR change in terms of SemVem - so not a breaking change. Is it true that every API change should be annotated as breaking? This seems counter-intuitive to me - thus I've removed that from the proposed merge message.