Skip to content

Conversation

@LukasReschke
Copy link
Member

No description provided.

@LukasReschke LukasReschke added the 3. to review Waiting for reviews label Aug 8, 2016
@LukasReschke LukasReschke added this to the Nextcloud 11.0 milestone Aug 8, 2016
@MorrisJobke
Copy link
Member

👍

@MorrisJobke
Copy link
Member

@josh4trunks Is it fine to update this NGINX config line:

  # Adding the cache control header for js and css files
  # Make sure it is BELOW the PHP block
  location ~* \.(?:css|js)$ {

@josh4trunks
Copy link
Contributor

josh4trunks commented Aug 8, 2016

looks good to me.
those extension types should be removed from the static location at the end (of the nginx config example)
location ~* \.(?:svg|gif|png|html|ttf|woff|ico|jpg|jpeg)$ {

@rullzer
Copy link
Member

rullzer commented Aug 8, 2016

👍

@LukasReschke LukasReschke merged commit 6791850 into master Aug 8, 2016
@LukasReschke LukasReschke deleted the also-cache-fonts-svg-and-gif branch August 8, 2016 16:41
@MorrisJobke
Copy link
Member

looks good to me.
those extension types should be removed from the static location at the end (of the nginx config example)

There is only one endpoint: preview.png that needs to be executed by PHP. Or am I wrong? cc @rullzer

@josh4trunks
Copy link
Contributor

josh4trunks commented Aug 8, 2016

@MorrisJobke that's why this block has to be below the PHP block, since regex blocks are matched first.

Actually, are we doing this? If so I don't understand how we know to pass preview.png to PHP based on the documented config.

@MorrisJobke
Copy link
Member

Actually, are we doing this? If so I don't understand how we know to pass preview.png to PHP based on the documented config.

These are the routes:

server/core/routes.php

Lines 112 to 115 in 8c7d7d7

$this->create('core_ajax_preview', '/core/preview')
->actionInclude('core/ajax/preview.php');
$this->create('core_ajax_preview', '/core/preview.png')
->actionInclude('core/ajax/preview.php');

@LukasReschke Where are the rules for excluding this in the apache config?

@MorrisJobke MorrisJobke self-assigned this Aug 9, 2016
@josh4trunks
Copy link
Contributor

ahh, yes this is handled correctly with the current nginx config. I was correct in my earlier statement, as long as the static location block is last. the nginx config will only match locations to actual files so since preview.png doesn't exist the request is given to index.php

MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 9, 2016
See nextcloud/server#769 for the corresponding Apache adjustment
@MorrisJobke MorrisJobke removed their assignment Aug 9, 2016
GitHubUser4234 pushed a commit to GitHubUser4234/server that referenced this pull request Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants