New methods to replace set(get)HtmlWithRange(Selection)

[Olek Nowodziński]

The idea is to implement methods, which would allow us to set HTML with ranges (sometimes selection) anchored in elements and text nodes, depending on the case. A corresponding methods to read such HTML with ranges (selections) should also be implemented.

Examples:

<p>{foo}</p> // range anchored in "foo" text node
<p>{}foo</p> // collapsed range anchored in "foo" text node
<p>[]foo</p> // collapsed range anchored in <p>
<p>{foo]</p> // range anchored in "foo" text node (start) and <p> (end)

Required by #11636.

[Piotrek Koszuliński]

cc

[Piotrek Koszuliński]

I pushed some changes to benderjs-ckeditor/t/12071.

1. Methods should not use "this" as bender.tools. Better to refer to bender.tools directly, so you can make a shortcut of such function (var make = bender.tools.setSelection)
2. I don't think that getRange should automatically use fixHtml. This decision should be left for a caller.
3. I've still got doubts about method names. For example getRange returns... an HTML. It's silly, but I think that we need to discuss this with the team, because all options seem to be equally bad.
4. There's no error if there is more than one range in getSelection(). We should make any test using getSelection() in such case fail clearly.
5. Tests for set/getSelection assumes that selection won't be normalised. I'm not sure you can do that. They fail on Opera (which does not yet have the same patch as Blink recently got), so they will also fail on Safari. I think that you want to know if getRange() matches /<p>[\{\[]foo[\}\]]</p>/ and that's it. You've got getRange() tested, so you can base get/setSelection() tests on it.
6. I would like to see more tests for setSelection firing selectionChange range. I've got doubt about a case when it doesn't really change selection (e.g. two subsequent identical setSelection calls). I think there should be no selectionChange then, but most importantly - it should be consistent.

[Olek Nowodziński]

Replying to Reinmar:

I pushed some changes to benderjs-ckeditor/t/12071.

1. Methods should not use "this" as bender.tools. Better to refer to bender.tools directly, so you can make a shortcut of such function (var make = bender.tools.setSelection)

Done.

2. I don't think that getRange should automatically use fixHtml. This decision should be left for a caller.

Done.

3. I've still got doubts about method names. For example getRange returns... an HTML. It's silly, but I think that we need to discuss this with the team, because all options seem to be equally bad.

This topic will be discussed once everything's done. It's not a blocker.

4. There's no error if there is more than one range in getSelection(). We should make any test using getSelection() in such case fail clearly.

Done.

5. Tests for set/getSelection assumes that selection won't be normalised. I'm not sure you can do that. They fail on Opera (which does not yet have the same patch as Blink recently got), so they will also fail on Safari. I think that you want to know if getRange() matches /<p>[\{\[]foo[\}\]]</p>/ and that's it. You've got getRange() tested, so you can base get/setSelection() tests on it.

That's right. Done.

6. I would like to see more tests for setSelection firing selectionChange range. I've got doubt about a case when it doesn't really change selection (e.g. two subsequent identical setSelection calls). I think there should be no selectionChange then, but most importantly - it should be consistent.

Pending...

[Piotrek Koszuliński]

Merged to master with ​git:6775bbee6 in ckeditor-dev (tests) and fe38a3d6 in ​https://github.com/benderjs/benderjs-ckeditor.