Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeslider breaks after 1000 revisions on HTML imported pads #5271

Closed
davue opened this issue Nov 10, 2021 · 8 comments
Closed

Timeslider breaks after 1000 revisions on HTML imported pads #5271

davue opened this issue Nov 10, 2021 · 8 comments
Labels

Comments

@davue
Copy link

davue commented Nov 10, 2021

Tested on:

  • Etherpad: 1.8.14
  • Browsers: Chrome 94.0.4606.81 and Firefox 94.0.1
  • Clients: Private instance and rich.etherpad.com
  • Node: 12.18.0

Steps to reproduce:

  1. Create a new pad and fill it with some content
  2. Export the pad as HTML
  3. Create a second pad and import the HTML from the first pad
  4. Create enough changes to make more than 1000 revisions

What I expected to happen:
The timeslider would work normally

What actually happened:
Revisions after 1000 are working but all revisions < 1000 simply show the content of revision 1000.

Additional Information

@JohnMcLear
Copy link
Member

JohnMcLear commented Nov 10, 2021

https://github.com/ether/etherpad-lite/blob/develop/src/tests/frontend/specs/timeslider_revisions.js -- note that this only creates 100 revisions. Steps you can probably take to move things forward

  • Extend timeslider_revisions.js to do a test with much more revisions (IE 1k+) and see if it fails in testing or not.

I would hazard a guess that the failure is due to the changeset loading method which probably needs to be rewritten as it's afaik code that hasn't been touched in several years.

TBH though I think we really need to think about the timeslider. I hate that it's a second rate citizen on it's own page and I'd strongly advocate moving the time-slider into the main pad editor and while we're at it expose more functionality for plugins such as the ability to replay the pad in real time... But these things are probably out of the scope of this bug.

@JohnMcLear JohnMcLear added the Bug label Nov 10, 2021
@webzwo0i
Copy link
Member

Could you test if applying #5253 fixes this problem?

@davue
Copy link
Author

davue commented Nov 16, 2021

Nope, this sadly does not fix the issue @webzwo0i

@webzwo0i
Copy link
Member

webzwo0i commented Nov 17, 2021

I tested it and for me it fixes the problem. Can you try and disable minify in the config (setting minify:false), go to a timeslider that has this bug, hit play and open the error console to see what exceptions are thrown?

The exception shown here is gone for me after applying #5253

insert@http://localhost:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=76afa96a:3680:21
(module ep_etherpad-lite/static/js/Changeset.js)/exports.mutateTextLines@http://localhost:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=76afa96a:3870:13
applyChangeset@http://localhost:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=76afa96a:5910:15
goToRevision@http://localhost:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=76afa96a:6002:36
update@http://localhost:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=76afa96a:6008:21

@davue
Copy link
Author

davue commented Nov 17, 2021

Ok you're right, this partially fixed the problem. 👍

At least for imports and the setHTML endpoint. I was testing it using the copyPadWithoutHistory endpoint which still breaks the timeslider.

Steps to reproduce:

  1. Copy an existing pad into a new one using copyPadWithoutHistory
  2. Create over 1000 revisions in the new pad
  3. Try to use the timeslider for revisions < 1000

Should I open up a new issue since this is not directly HTML related but specific to pad copying?

Edit:
This is the only error visible in the log when this happens:

[2021-11-17 14:12:44.288] [WARN] console - failed to compose cs in pad: g.tySn8Q2Aogt2SBWO$17835229-f8b2-4736-b137-36fdf82b04f7  startrev: 0  current rev: 2
[2021-11-17 14:12:44.288] [ERROR] console - Error while handling a changeset request for g.tySn8Q2Aogt2SBWO$17835229-f8b2-4736-b137-36fdf82b04f7 Error: Failed assertion: mismatched composition of two changesets { start: 0, granularity: 10, requestID: 11367 }

@davue
Copy link
Author

davue commented Jan 18, 2022

It looks like that copyPad works without any issues. Only copyPadWithoutHistory seems to be affected by this bug.

Is there another way to clear the history of a pad to have a workaround?

@SamTV12345
Copy link
Member

@Gared Is creating a fix for that so you can remove the history of a pad and its revisions

@SamTV12345
Copy link
Member

Gared just checked locally with 25k versions. It seems like this could be caused by a corrupt pad which occurs when Etherpad crashes and hasn't finished writing to the database or the server crashes during a transaction. cc @Gared

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants