Add endpoint for public editor previews

[rullzer]

Simple endpoint for single file shares to get an editor preview:

so: <server>/apps/files_texteditor/public/<token> gives you back what is required.

@MorrisJobke @icewind1991 could one of you look into the fancy javascript stuff?

[MorrisJobke]

@MorrisJobke @icewind1991 could one of you look into the fancy javascript stuff?

What is needed?

[rullzer]

Well, we need to load (in read only for now) the editor. So that it will display the preview in the editor (just like it does in the sidebar).

Else we can't display non utf-8 (and non ascii) encoded text files properly.

[MorrisJobke]

Nothing for 13 it seems -> moving

[danxuliu]

I have rebased onto current master and added the needed changes to show the plain text preview.

However, as it is read-only, I just used (almost) the same code as for the default plain text previews instead of bothering to add the editor in read-only mode ;-) Basically it is the same as the default plain text previews, but with better encoding support (provided by the public endpoint added by @rullzer).

A better long term solution would be to refactor the sidebar previews and the public previews to a generic preview system, but for now these changes fix the bug and should be easily backporteable ;-)

[rullzer]

@danxuliu ok so no editing if you share it with write permissions yet ;)

[danxuliu]

Mmm, it seems that the Range header can not be used with the public endpoint, because it returns JSON instead of the plain text file.

[rullzer]

@danxuliu o that I can fix if needed ;)

[danxuliu]

@rullzer Please :-)

[rullzer]

@danxuliu done.

[rullzer]

Should we maybe request more than 1 kb? Like maybe 20 or so max?

I see double (..)

I can't select the text (which would be nice to do)

[danxuliu]

@rullzer Mmm, it does not work here. But it seems to work in your screenshot... although I do not know why :-P

If I am not mistaken, the range affects the whole response contents, not only the file contents. Thus, although the file contents length is limited to the range, the response is still cut before the end and the JSON parsing fails; the whole response would be {"filecontents":" followed by 1000 characters of the file and then ", 'writeable'...}, but once the range is applied it ends being {"filecontents":" followed by (1000 - length of {"filecontents":") characters of the file. Or am I missing something?

Should we maybe request more than 1 kb? Like maybe 20 or so max?

I used just 1KB because that is what is used in the default plain text preview. I have no preference in that regard, feel fee to change it if you want ;-)

I see double (..)

I have noticed that (...) sometimes appears when it should be hidden; I intended to fix that in a following pull request because it is not caused by the changes made here. However I have never seen it double :-S

I can't select the text (which would be nice to do)

Selecting the text was explicitly disabled when default plain text previews were added. This is just a guess, but I think that it was disabled due to the height limit of the preview. If the browser window is short and a large text preview is shown the bottom part of the text would not be visible, but if you select the text by dragging down until the Download button even the text that is not visible would be selected, which could be a bit surprising for the user.

[danxuliu]

@nextcloud/designers Some questions about plain text file previews:

• How large should the preview be? Right now 1000 bytes are shown, which is 1000 ASCII characters, but for non-latin scripts it could be 500 characters or even less
• Right now the preview is cropped so the Download button is always visible (at least, if you do not resize the window; if the window is resized the preview height is not updated, which could be seen as a bug). Would it be better to show the preview in its full height, even if the Download button ends below the fold, like it is done with Markdown previews?
• Related to the previous question, right now the text in a plain text preview can not be selected. I guess that it was disabled due to the height limit, because if the text is cropped and you select the text by dragging down until the Download button even the text that is not visible would be selected, which could be a bit surprising for the user. Should the text be selectable even if the text is cropped? Or given that it is just a preview it should not be selectable even if shown in full height (although personally I would make it selectable in this case)?
• To show the Download button above the fold and also make possible to see the full height preview and select the text, maybe crop the preview when the page is opened, and as soon as the user clicks on it expand it to its full height? How to make the user aware that that will happen? Use a dimmed text when the preview is cropped and use the full opacity on hover (and always when expanded)?

[MorrisJobke]

Right now the preview is cropped so the Download button is always visible (at least, if you do not resize the window; if the window is resized the preview height is not updated, which could be seen as a bug). Would it be better to show the preview in its full height, even if the Download button ends below the fold, like it is done with Markdown previews?

The download button will be (is already?) moved to the header anyways.

Related to the previous question, right now the text in a plain text preview can not be selected. I guess that it was disabled due to the height limit, because if the text is cropped and you select the text by dragging down until the Download button even the text that is not visible would be selected, which could be a bit surprising for the user. Should the text be selectable even if the text is cropped? Or given that it is just a preview it should not be selectable even if shown in full height (although personally I would make it selectable in this case)?

It should be selectable IMO - @nextcloud/designers

[rullzer]

So the download button could even go I guess since we have that on the top now.

[rullzer]

Anyway I'd say lets get this in now and fix more stuff in later PR's.

[danxuliu]

@rullzer

Anyway I'd say lets get this in now and fix more stuff in later PR's.

Sure... once the range is fixed ;-) (I have checked again and it fails as explained in the first part of that comment; I still do not know why it works for you :-S Cached JavaScript, maybe?)

[rullzer]

Maybe it is your webserver...

[rullzer]

Of course for a read only endpoint we could just return the text (not in json) as it makes no real sense anyway. If we want at some point to have a real editor then we need to return the whole text anyways...

[danxuliu]

@rullzer

Maybe it is your webserver...

You are right, it was related to my webserver.

With the PHP built-in webserver the preview loads, but with Apache it does not (at least, with the official Docker image for Nextcloud and the official Docker image for PHP on Apache (which the Nextcloud one is based on); I have not tested it in other environments).

However, it seems that the reason that it works with the PHP built-in webserver is simply because it ignores the Range header, so it returns exactly what was returned by the controller with a 200 HTTP status. Apache, on the other hand, honours the header, so it returns a subset (the range of bytes defined in the header) of what was returned by the controller with a 206 HTTP status. As explained above, although in the controller the range is applied to the file contents the response body is larger due to the extra JSON data, and thus it ends being cut by Apache.

Having said that, I have noticed that although the sidebar preview uses a Range header too to request just the beginning of text files even in Apache the whole file is returned (and the rest of request headers seem to be the same, except for Connection: keep-alive that is included in the sidebar preview but not in the public preview). I have no idea why it does not honour the Range header here...

Anyway, in order to return JSON and also use the Range header it seems that the controller has to apply the range to the whole returned JSON and not only to the contents of the file.

Another option would be to somehow prevent the webserver to apply the Range header on its own when it was already applied by the controller.

And another option would be to remove the Range handling from the controller and handle the 206 HTTP status in JavaScript (although in this case the whole file would be downloaded depending on the webserver, and also for files larger than the threshold an additional request would have to be done to the server to get the end of the JSON data).

Of course for a read only endpoint we could just return the text (not in json) as it makes no real sense anyway. If we want at some point to have a real editor then we need to return the whole text anyways...

Returning the text itself would easily solve all of the above, so fine by me ;-) However, if at some point a real editor is added I guess that the JSON response would be needed (for example, to know whether the file is writable or not), so all of the above would need to be taken into account again.

[jancborchardt]

As @MorrisJobke said, the text should be selectable. Just like we do for Markdown shares, directly show the text in nice formatting so you can quickly share notes. Show as much of the text as possible – ideally everything.

[MorrisJobke]

As @MorrisJobke said, the text should be selectable. Just like we do for Markdown shares, directly show the text in nice formatting so you can quickly share notes. Show as much of the text as possible – ideally everything.

For markdown we also just show the first 512 KB or so, but this is already quite some text (~70k to 80k words).

[rullzer]

@danxuliu well if we allow real editing we can't do range requests anyway.

I'll convert the controller later.

[rullzer]

@danxuliu please test again.

[danxuliu]

@rullzer The Content-Type header of the response is application/json; charset=utf-8 (at least, with Apache; I have not tested with other web servers), so it still fails.

[rullzer]

@danxuliu now it is set as text-plain.

However now I see no newlines (I just see \n). So maybe we need some editor magic then.

[MorrisJobke]

@danxuliu Could you have a look at this?

[danxuliu]

Tested and works 👍

Pending issues mentioned in comments will be fixed in other pull requests.

[rullzer]

@MorrisJobke please review as well

[MorrisJobke]

Tested and works 👍

[SurajVerma]

Sorry to bump in too late, but it is still not working for me ? Any work around for this ? I don't want user to download the text file, I just want them to view it and if needed edit online.