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

Revert for files app to use ajax download instead of Webdav #23521

Closed
wants to merge 1 commit into from

Conversation

PVince81
Copy link
Contributor

This is required to be able to show a correct HTML error page in case
of error. Webdav only returns XML.

Fixes #23500

@DeepDiver1975 @oparoz @SergioBertolinSG @icewind1991 @rullzer @nickvergessen

This is required to be able to show a correct HTML error page in case
of error. Webdav only returns XML.
@PVince81 PVince81 added this to the 9.1-current milestone Mar 23, 2016
@PVince81
Copy link
Contributor Author

@karlitschek backport to stable9 ? Without this the user would see an XML response in case or an error page (ex: firewall)

@DeepDiver1975
Copy link
Member

Without this the user would see an XML response in case or an error page (ex: firewall)

is that really of an issue?

@PVince81
Copy link
Contributor Author

@DeepDiver1975 it's not user-friendly to dump XML content instead of an error page instead of a nice message "You cannot download this file because it was blocked" or "Cannot decrypt this file", etc.

Unfortunately there isn't much that can be done directly. One would need to pre-download the file with $.ajax to see if there's an error and if there is not, redirect to the link. Ugly.

@LukasReschke
Copy link
Member

Unfortunately there isn't much that can be done directly. One would need to pre-download the file with $.ajax to see if there's an error and if there is not, redirect to the link. Ugly.

What about doing a HEAD request first and if statuscode !== 200 then redirect to some kind of error page?

@PVince81
Copy link
Contributor Author

What about doing a HEAD request first and if statuscode !== 200 then redirect to some kind of error page?

Hmmm, interesting. That could work, assuming that HEAD will also fail for encryption, firewall and other possible errors.

Tbh I'm not willing to experiment too much given the remaining short time for 9.0.1, the ajax/download.php isn't that bad considering that we also use it for folder downloads.

I'm happy to do more experimentation for 9.1 (HEAD or provide a Sabre HTML rendered like the BrowserPlugin)

@PVince81
Copy link
Contributor Author

I'll still have a quick try with HEAD because you picked my curiosity 😉

@DeepDiver1975
Copy link
Member

If we really want a fancy web page displayed we can add a plugin which will convert the exception into a proper page for Browsers.
Should be possible with some User Agent magic.

@PVince81
Copy link
Contributor Author

Yeah, I think in the end I'd also prefer the Sabre plugin approach, because we'll need it again with the future zip download endpoint, where a HEAD request cannot predict fully whether the request can succeed (ex: locked files, etc)

@DeepDiver1975
Copy link
Member

Yeah, I think in the end I'd also prefer the Sabre plugin approach,

I can take care about this later today if you want ...

@PVince81
Copy link
Contributor Author

@DeepDiver1975 you're welcome to wield the Sabre today 😄

Maybe the official BrowserPlugin can be used as inspiration.

@LukasReschke
Copy link
Member

From what I get the browser plugin magic is there => Close.

@LukasReschke LukasReschke deleted the files-reverttoajaxdownload branch April 1, 2016 09:20
@lock
Copy link

lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error page is missing, just a xml appear.
3 participants