Problem/Motivation

The CSS optimizer updates the url paths inside the css by setting those relative to the drupal root folder (instead to the css file). Unfortunatly it does not include the real base path (just '/'), which leads to wrong paths when drupal is installed inside a subfolder of the webroot.

Proposed resolution

Prepend base path instead of backslash.

User interface changes

none

API changes

none

Data model changes

none

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pminf created an issue. See original summary.

pminf’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
744 bytes

I've also removed the condition if the directory is just '.', because this is never true when base path (or even '/') is prepended before.

pminf’s picture

Assigned: pminf » Unassigned
Status: Needs review » Needs work

Sorry, but my last patch does not fix this issue. The path is still incorrect.

This is my unminified css before preprocessing with advagg:

.icon-foo {
  background-image: url("../img/icon-foo.svg");
}

(the img folder is next to the css folder)

This is my unminified css after preprocessing with advagg (without patch #2):

.icon-foo {
  background-image: url("/sites/default/themes/custom/my_theme/img/icon-foo.svg");
}

And this is my unminified css after preprocessing with advagg with patch #2:

.icon-foo {
  background-image: url("/subdirectory/drupal/sites/default/themes/custom/my_theme/dist/css../img/icon-foo.svg");
}

The double dot is still part of the path. I'm not a regex expert, so I don't know how to fix this:

return preg_replace('/url\(\s*([\'"]?)(?![a-z]+:|\/+)([^\'")]+)([\'"]?)\s*\)/i', 'url(\1' . $directory . '\2\3)', $contents);
pminf’s picture

Status: Needs work » Needs review
FileSize
524 bytes

OK, I managed to fix the double dot issue and it was not because of the regex pattern. The following part is still needed:

    // If the file is in the current directory, make sure '.' doesn't appear in
    // the url() path.
    $directory = $directory == '.' ? '' : $directory . '/';

Could somebody explain why this line is necessary? And why are the urls updated anyway?

NickDickinsonWilde’s picture

Assigned: Unassigned » NickDickinsonWilde
Status: Needs review » Needs work
Issue tags: +Needs tests

Delayed release :) thanks for catching that. I'll answer that question about that line shortly... I can't say I remember/understand it this moment.
For the 8.x-3.x branch, I'm doing tests for any bug fixes; no test for that method yet actually, so putting a few tests there and then I'll apply your patch, thanks.

NickDickinsonWilde’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
4.01 KB

Test that *should* fail.

NickDickinsonWilde’s picture

hmmm not sure why that didn't fail given the information I have on the test runner; well that said, the test is working correctly on my local so I'll do it for now and if further fails occur I'll investigate improving the test.

Made a bit of changes from your patch and committing that and the tests.

In regards to the:
$directory = $directory == '.' ? '' : $directory . '/';
bit of code, refreshed my memory and don't really need that in that form. Left over from Drupal 7 when the asset could be at the site root. No longer the case, but any directory will need to end in '/' or won't work. So getting rid of that line but ensuring that the directory ends in with a slash.

Since I did do changes from your patch, so let me know if it no longer works.

Thanks again for the contribution 😃

NickDickinsonWilde’s picture

Assigned: NickDickinsonWilde » Unassigned
Status: Needs review » Fixed

  • NickWilde committed be0d7b4 on 8.x-3.x
    Issue #2892640 by pminf, NickWilde: CSS optimizers url update does not...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.