Fix text lines mutator

[webzwo0i]

This should address #2836 but please do not merge this yet!
As you can see there are some "TODOs" left.

The textLinesMutator gets lines of text and a changeset and applies the latter to the former. It should work for attribution and normal text lines. Sometimes when there are no lines left but the changeset is not finished it will crash.

I am not sure I did this right, but I will try to solve the remaining TODOs. I want to ask if anyone has a good idea how to test this stuff. It is used in the timeslider and on clients before submitting changesets. What I need is working test data:

• take a running epl instance and dozens of html (from different revisions) and etherpad exports
• the html exports represent text lines
• randomly take two html exports and calculate composed changesets in both directions using the revisions from etherpad exports
• apply the composed changesets to the html export's lines and test for equality with the other html export

Does this sound reasonable to you? Any other ideas how to be sure this does not break anything?

[webzwo0i]

note to self: this breaks some pads with TypeError: rep.lines.atIndex(...) is undefined

[JohnMcLear]

Be happy to see this land :) Sounds like it's already fixing things? Can I help by giving you access to beta.etherpad.org ?

[muxator]

Hi, this is an old PR, but it seems important. Any chance to resume the work?

[JohnMcLear]

@muxator I tried to bump @webzwo0i - this defo looks worth putting some time into..

[JohnMcLear]

I spoke to @webzwo0i he's going to work on this when he can :) so we're waiting on him to review/merge.

[webzwo0i]

Will submit the documentation from here in a separate PR. The tests I added to easysync_tests in this PR are probably nonsense (the last newline is special). textLinesMutator will crash on some crafted changesets but the way it is used in the timeslider seems to be save and every exception is handled well

[webzwo0i]

Reopening so that I'm reminded I need to make a new PR with docs.
Also I see a related error that renders the timeslider useless. Steps to replicate:

1. create a pad
2. enable allowAnyoneToImport or write something
3. import an empty html file (or anything else that calls setText("\n"))
4. goto timeslider, disable "follow contents" and go backwards, it won't work between revision 2 and 1 (revision 1 in my case is "write something" from step 2. so it triees to apply the inverse of the changeset that is created with setText("\n"))
   timeslider.js?callback=require.define&v=33c0661d:3378 curSplice[sline] not populated, actual curSplice contents is (3) [0, 2, undefined] . Possibly related to https://github.com/ether/etherpad-lite/issues/2802 timeslider.js?callback=require.define&v=33c0661d:3380 Uncaught TypeError: Cannot read property 'substring' of undefined at Object.insert (timeslider.js?callback=require.define&v=33c0661d:3380) at Object.exports.mutateTextLines (timeslider.js?callback=require.define&v=33c0661d:3552) at applyChangeset (timeslider.js?callback=require.define&v=33c0661d:6007) at Function.goToRevision (timeslider.js?callback=require.define&v=33c0661d:6099) at Array.goToRevisionIfEnabled (timeslider.js?callback=require.define&v=33c0661d:6309) at _callSliderCallbacks (timeslider.js?callback=require.define&v=33c0661d:6401) at setSliderPosition (timeslider.js?callback=require.define&v=33c0661d:6470) at HTMLButtonElement.<anonymous> (timeslider.js?callback=require.define&v=33c0661d:6690) at HTMLButtonElement.dispatch (ace2_common.js?callback=require.define&v=33c0661d:5492) at HTMLButtonElement.elemData.handle (ace2_common.js?callback=require.define&v=33c0661d:5300)

Coverage test needs to go into #4190

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
1 Code Smell

No Coverage information
0.0% Duplication

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging 8ea5033 into 4637b2b - view on LGTM.com

new alerts:

• 1 for Syntax error

fixed alerts:

• 1 for Unused variable, import, function or class

[rhansen]

I rebased onto current develop and split the documentation changes into a separate PR (#5199).

[webzwo0i]

Thanks @rhansen for the cleanup work!!!
Before this can be merged, I need to finish up #4190

[rhansen]

Rebased onto latest develop.

[webzwo0i]

Closed in favor of #5253