Closed (fixed)
Project:
Advanced CSS/JS Aggregation
Version:
8.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
6 Jul 2017 at 11:34 UTC
Updated:
22 Jul 2017 at 04:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
pminfI've also removed the condition if the directory is just '.', because this is never true when base path (or even '/') is prepended before.
Comment #3
pminfSorry, but my last patch does not fix this issue. The path is still incorrect.
This is my unminified css before preprocessing with advagg:
(the img folder is next to the css folder)
This is my unminified css after preprocessing with advagg (without patch #2):
And this is my unminified css after preprocessing with advagg with patch #2:
The double dot is still part of the path. I'm not a regex expert, so I don't know how to fix this:
Comment #4
pminfOK, I managed to fix the double dot issue and it was not because of the regex pattern. The following part is still needed:
Could somebody explain why this line is necessary? And why are the urls updated anyway?
Comment #5
nickdickinsonwildeDelayed 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.
Comment #6
nickdickinsonwildeTest that *should* fail.
Comment #7
nickdickinsonwildehmmm 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 😃
Comment #8
nickdickinsonwilde