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

Misuse of "Import" erases all history data #5213

Closed
jej opened this issue Oct 1, 2021 · 27 comments · Fixed by #5250
Closed

Misuse of "Import" erases all history data #5213

jej opened this issue Oct 1, 2021 · 27 comments · Fixed by #5250
Assignees

Comments

@jej
Copy link

jej commented Oct 1, 2021

Describe the bug
I see some closed issues about this, but maybe it worths to reopen it.
I suggested to a friend to use etherpad for a collaboration work. After few days and many modifications, she wanted to import an inline image in the text. She clicked on "Import/Export", picked an image file and didn't care about the warning... Of course the pad was scrambled by a binary text, and the image was not inserted inline as expected by the user. The problem is that now the historic does not show the revisions any more. The revisions are browsable with the slider, but current content only is displayed. All previous revisions seems recorded as usual, but not available.
I tested this on https://framapad.org/fr/ . I don't know what version of etherpad they use.

To Reproduce
Steps to reproduce the behavior:

  1. Write/modify some text, in order to create some revisions.
  2. Click on 'Import/Export'.
  3. Pickup a binary image file (ex. png/jpg)
  4. Import it, acknowledge the warning.
  5. The binary data replace the text. Eventually: erase it and add some text.
  6. Look at the history to see if previous revisions are still displayed.

Expected behavior
It's not a problem if binary data erase the current version, but at least it should be possible to retrieve the previous content of the pad.
Maybe the GUI should be more careful with import. A mandatory checkbox could make the user aware that all the content will be replaced, instead of a simple button.

Server (please complete the following information):

@JohnMcLear
Copy link
Member

  1. There is a plugin for forking pads to a new state from a point in time. ep_fork maybe? You access the previous revision through the timeslider. Plugins are visible on the plugin list page.

  2. Re importing ux, I would say the expected ux for import in any text editor is to replace the whole document no?

@jej
Copy link
Author

jej commented Oct 2, 2021

Thanks for your reply.

  1. I didn't know this plugin. I will test for my own use.
  2. The problem here is the bug about the timeslider. If the import replace the doc, why the slider allows to browse the revisions but don't show them? I would say that import a doc just create a new revision with the new content. But here the concept of the pad is broken...

@JohnMcLear
Copy link
Member

  1. Gimme 5 mins will find it for you.
  2. Import should create a new revision, you are right! Can you provide exact steps to replicate then here because if you are saying it's not creating a new revision something is very broken! Please provide steps to replicate on a clean install / no plugins etc.

@JohnMcLear
Copy link
Member

  1. https://www.npmjs.com/package/ep_copypad -- give this a try, I'm not sure why it's not visible in npm / plugin list tho! You would need to install via npm install ep_copypad though as it isn't visible in the plugin list for some reason..

@jej
Copy link
Author

jej commented Oct 2, 2021

To Reproduce
Steps to reproduce the behavior:

  1. Write/modify some text, in order to create some revisions.
  2. Click on 'Import/Export'.
  3. Pickup a binary image file (ex. png/jpg)
  4. Import it, acknowledge the warning.
  5. The binary data replace the text. Eventually: erase it and add some text.
  6. Look at the history to see if previous revisions are still displayed.

@JohnMcLear
Copy link
Member

JohnMcLear commented Oct 2, 2021

RE replicating btw, I tried to replicate locally but I get an error Import failed: The upload failed, please try again

I get warning [2021-10-02 10:47:01.610] [WARN] ImportHandler - Attempt to import non-ASCII file

My assumption here is you have an instance with a modified settings.json and something like abiword or soffice enabled?

@JohnMcLear
Copy link
Member

Please provide STR in a clean environment of Etherpad, not a modified one.

@jej
Copy link
Author

jej commented Oct 2, 2021

Yeah that's the problem, I don't manage the platform Framapad. I guess it's not so modified. My own version is full of plugins...
Do you know a clean fresh install I can try?

@JohnMcLear
Copy link
Member

You can spin one up using the docs really quickly/easily, that's probably the best approach to ensure it's clean and we can replicate?

@JohnMcLear
Copy link
Member

Just a thought. Image size could be maxing out buffer causing timeslider to not work? An idea, not researched

@jej
Copy link
Author

jej commented Oct 2, 2021

Ok... I can reproduce with a clean install :

  1. Write/modify some text, in order to create some revisions.
  2. Click on 'Import/Export'.
  3. Pickup a HTML file.
  4. Import it, acknowledge the warning.
  5. The HTML content replace the text. (Eventually: erase it and add some text.)
  6. Look at the history to see if previous revisions are still displayed.

In my case, the slider can browse the revisions but nothing is displayed. If I CTRL+SHIFT+R the page, the current content is displayed for any revisions.

@JohnMcLear
Copy link
Member

ack, gimme 5 will check

@JohnMcLear
Copy link
Member

  1. I see previous revisions so unable to replicate. Do you get any errors in console?

@JohnMcLear
Copy link
Member

What size is the HTML file contents?

I was able to get curSplice[sline] not populated, actual curSplice contents is Array(3) [ 0, 2, undefined ] . Possibly related to https://github.com/ether/etherpad-lite/issues/2802

I think STR for that is insert content, import empty HTML file, type some stuff, visit timeslider, click play.

@JohnMcLear
Copy link
Member

JohnMcLear commented Oct 2, 2021

I think the bug here is #5214 - I made a new issue as to remove the noise. Please can you confirm that in your developer console you see the same error as above?

@jej
Copy link
Author

jej commented Oct 2, 2021

The HTML file is 18ko.
If I debugRun.sh it works as expected. But if I run.sh I get the bug.
I don't see any error.

[2021-10-02 12:34:32.900] [INFO] access - [CREATE] pad:testpad socket:6VP8nWdfr5ZxrCu0AAAD IP:127.0.0.1 authorID:a.m0Om9fc1qS3Fgmur
[2021-10-02 12:35:57.571] [WARN] console - Ignoring unsupported elements (you might want to install a plugin): header, h1, a, nav, aside, article, table, tbody, tr, th, td, img, section, h2, footer, address
[2021-10-02 12:36:01.854] [INFO] access - [LEAVE] pad:testpad socket:6VP8nWdfr5ZxrCu0AAAD IP:127.0.0.1 authorID:a.m0Om9fc1qS3Fgmur
[2021-10-02 12:36:02.888] [INFO] access - [ENTER] pad:testpad socket:4_26Ns5UNNFp4Y5BAAAE IP:127.0.0.1 authorID:a.m0Om9fc1qS3Fgmur

Maybe something related to browser's cache...

@JohnMcLear
Copy link
Member

Gimme 5 will test with run.sh

@JohnMcLear
Copy link
Member

JohnMcLear commented Oct 2, 2021

I can still access old revisions(it works fine other than errors in console) when using run.sh but do experience the error in #5214 which I wonder if we publish a fix for that might fix your issue? Either way Etherpad is designed to never lose document fidelity, history or integrity, it's kind of a big thing 📦

Can you clear the browser cache and test between run.sh and debugRun.sh

@jej
Copy link
Author

jej commented Oct 2, 2021

Ok, I cleared. But I didn't think to look at JS console log, sorry...

When I move the revisions slider I get this for each move:

Uncaught TypeError: $(...)[0] is undefined
    a http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:70
    a http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:70
    u http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:70
    loadBroadcastJS http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:70
    n http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:73
    h http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:73
    loadBroadcastSliderJS http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:73
    dispatch http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    handle http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    add http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    we http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    each http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:27
    each http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:27
    we http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    on http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    t http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    loadBroadcastSliderJS http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:73
    dispatch http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    handle http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    add http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    we http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    each http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:27
    each http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:27
    we http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    on http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    t http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=a7da73b0:38
    loadBroadcastSliderJS http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:73
    handleClientVars http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:3
    init http://0.0.0.0:9001/javascripts/lib/ep_etherpad-lite/static/js/timeslider.js?callback=require.define&v=a7da73b0:3
    emit http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    onevent http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:8
    onpacket http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:8
    exports http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:8
    emit http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    ondecoded http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    exports http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:8
    emit http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    add http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    ondata http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    exports http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:8
    emit http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    onPacket http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    setTransport http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    emit http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:6
    onPacket http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:7
    n http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:7
    decodePayload http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:7
    onData http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:7
    doPoll http://0.0.0.0:9001/socket.io/socket.io.js?v=a7da73b0:7
timeslider.js:70:2365

@JohnMcLear
Copy link
Member

Ahhhh, I bet you see a different error because of minify, let me confirm that.

And just to be 100% sure, you have no plugins installed, correct?

@jej
Copy link
Author

jej commented Oct 2, 2021

I just followed the quick install from the git. No plugin, no setttings tweaks, just the basic install.

@JohnMcLear
Copy link
Member

Hrm, nope, with minify true I still get the curSplice error.

Either way we're firing an error after this edit event, so let's get that resolved first then you can git pull and see if it's fixed?

@JohnMcLear
Copy link
Member

https://discord.gg/CWQXkG7K for Discord btw

@JohnMcLear JohnMcLear self-assigned this Oct 2, 2021
@jej
Copy link
Author

jej commented Oct 2, 2021

Ok perfect. I keep my install for the following investigations!
Thanks for your support.

@JohnMcLear
Copy link
Member

After some more play I got the error Uncaught TypeError: $(...)[0] is undefined.

I deleted all pad contents, did an import of a .html file (which imported blank) and now I get the error. I assume it's related to line numbers.

@rhansen
Copy link
Member

rhansen commented Oct 29, 2021

This should be fixed by PR #5250.

@rhansen rhansen closed this as completed Oct 29, 2021
@webzwo0i
Copy link
Member

Unfortunately, there are probably two bugs. The "Uncaught TypeError" should be fixed by #5250.
The curSplice problem is a duplicate of #5214 and should be fixed when #2911 is ready

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

Successfully merging a pull request may close this issue.

4 participants