Data export doesnt work on Safari

Tangential note, because I found this issue queue while looking for why my generated csv files were not downloading but opening as text in Safari.

I needed to add AddType application/octet-stream csv to my apache config (tested/proven) (or add_header Content-disposition "attachment; filename=$1"; in Nginx (not personally tested yet)).

Looks like the views_data_export module goes one of two routes: If Drupal/php serves the csv file download response directly, the content header text/csv and content-disposition to download is added in php script. But when Drupal runs the batch build of the csv for download, it writes the csv file to disk, and when the client requests this file by its uri, Drupal/php are no longer consulted: The web server is in charge of headers here.

Back to this issue: There is a switch in there that checks whether a filename is specified in the View, and text/csv etc headers are only added if so. Specify a filename, you'll be good to go: this looks intentional like if some consumer wanted to import data as csv from a view. Maybe content-disposition confuses the spider?

I experienced this same issue with Safari showing the CSV contents in the browser versus initiating a download of the CSV file in a View batch process.

I noticed the Content-Disposition was being set as "inline" so adjusted response to "attachment" in an Event Subscriber where the Uri for the event ended a csv extension. Drupal was still processing the CSV file download, not directly from the server, since it checks the user downloading was the one that created it.

  /**
   * Implements \Symfony\Component\EventDispatcher\EventSubscriberInterface::getSubscribedEvents().
   */
  static function getSubscribedEvents() {
    $events[KernelEvents::RESPONSE][] = array('onRespond');
    return $events;
  }

  /**
   * Adjustment to force a download for csv in Safari.
   *
   * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event
   */
  public function onRespond(FilterResponseEvent $event) {
    if (substr($event->getRequest()->getUri(), -4) === '.csv') {
      $response = $event->getResponse();
      $response->headers->set('Content-Disposition', 'attachment');
    }
  }

The issue takes place when export files are stored to the private location. Hook "hook_file_download()" allows add headers to private files. Patch below adds the hook to the module. Also with this change JSON download also works in Safari.

I'm not sure what encoding is needed for filename inside of "Content-disposition" header, in Chrome and Safari it works with backslashes, URL encode and plane, even if filename has quotes. So I keep it plain, same to implementation in the module in DataExport::buildStandard()

I have this problem but I'm not using the private file system (only public), so the patch doesn't help me. I tried the event subscriber in #3, and while it did work, it seems a little overbearing to run that if statement on every page request across the site on the off chance that the request contains "csv". Adding to .htaccess as suggested in #2 didn't work for me.

In the end I did what was suggested by the reporter: Set the file name in the views path settings.

This is a bug for sure, but at least it's easily worked around.

I created a merge request based on @dewalt's patch and committed with them as the author of the commit.

For a while now, using this module, we have seen inconsistent report downloads -- sometimes they would download instantly, sometimes they wouldn't and the user would need to click the link to download the file.

The other issue we found that led me to this issue to begin with, was that in Safari, after generating an export, Safari would present the user with the JSON blob of content, which is probably scary to a user as it's JSON and doesn't look like the Drupal page anymore.

This patch indeed fixes that issue where the file is downloaded and they remain on the Drupal theme.

So, this patch resolves 2 issues for us:
1. The Safari file download issue with JSON blob
2. The automatic download functionality

I fixed the merge conflict so the tests pass again.

Setting to needs work because of my comment on the MR.

Thanks. I confirmed your analysis @steven jones, and made a suggestion on the merge request.