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

Reports: Downloaded XLS file now has the correct file suffix #697

Closed
wants to merge 1 commit into from
Closed

Reports: Downloaded XLS file now has the correct file suffix #697

wants to merge 1 commit into from

Conversation

PerplexedTheta
Copy link

Please see issue 695

@PerplexedTheta
Copy link
Author

Hi,

Any update on this request?

Thanks,
Jake

@andyp-uk
Copy link
Contributor

What you've done looks fine. Someone will need to test then merge it. If nobody else gets to it first, I will very likely do this myself before the next release.

@andyp-uk
Copy link
Contributor

andyp-uk commented Mar 29, 2022

Having a look at the issue myself, I see the file generated isn't actually an XLS file. Adding an XLS file suffix makes it easier for users to open the file with Excel, but there is still a warning that the file could be corrupted or unsafe.

Other members of the CORAL steering committee might want to give their opinion before this is merged.

@gmcharlt
Copy link

While looking at this, I ran across the following: back in 2016, Microsoft released a security update that made Excel refuse to load HTML files that were given an .xls extension at all (unless the website was added to a trusted list). Microsoft subsequently changed the behavior so that such files would instead be opened in "protected view". Consequently, there are theoretically some users who may not be able to open such files in Excel.

In any event, I do get a warning from Excel that the file may be corrupted. Better long term options might be to provide a CSV export option and/or a library to generate true XLS or XSLX output.

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 this pull request may close these issues.

3 participants