canton7 / fuelphp-casset

Better asset management library for fuelphp (with minification!)
MIT License
103 stars 29 forks source link

Remote assets beginning with // fail to load #43

Open benswinburne opened 11 years ago

benswinburne commented 11 years ago

When loading remote assets using just // instead of http(s):// causes the following issue

ErrorException [ Warning ]: file_get_contents(//fonts.googleapis.com/css?family=Open+Sans:300,400,600): failed to open stream: No such file or directory

Adding the following code to load_file() resolves the problem.

if (strpos($filename, '//') === 0)
{
    $protocol = \Input::server('https', false) ? 'https:' : 'http:';
    $filename = $protocol.$filename;
}
canton7 commented 11 years ago

Protocol-relative URLs make sense for CSS files which are requested by the browser, as they avoid "This page contains both secure and non-secure items" warnings.

However I can't see any advantage at all for remote assets which are requested by the server, then cached locally on the server before being delivered to the client. When making this request, the server shouldn't care at all whether the client requested the page over http or https - it has absolutely no bearing on whether the server should then request the asset from its remote location over http or https.

Actually, did you read (and understand!) the "Remote files" section in the README? It states that for a remote file to be linked to directly in the client - rather than being fetched from its source by the server - combining must be disabled. Is this what caught you out?

benswinburne commented 11 years ago

Well I was adding the files in a controller on the fly so I couldn't work out how to prevent minification and combining of this particular file.

\Casset::add_path('googleWebFonts', array(
 'path' => '//fonts.googleapis.com/',
 'css_dir' => 'css',

)); \Casset::css('googleWebFonts::?family='.$names);

But the problem was encountered because in development mode we don't have combine turned on, so assets are added with just // but in production, combine and minify are turned on so the original ticket was to allow for switching. Granted this wouldn't be a problem if I was able to prevent minification etc for this asset in the controller?

Would I need to add and enable a whole group with a specific path to do this?

On 24 June 2013 18:28, Antony Male notifications@github.com wrote:

Protocol-relative URLs make sense for CSS files which are requested by the browser, as they avoid "This page contains both secure and non-secure items" warnings.

However I can't see any advantage at all for remote assets which are requested by the server, then cached locally on the server before being delivered to the client. When making this request, the server shouldn't care at all whether the client requested the page over http or https - it has absolutely no bearing on whether the server should then request the asset from its remote location over http or https.

Actually, did you read (and understand!) the "Remote files" section in the README? It states that for a remote file to be linked to directly in the client - rather than being fetched from its source by the server - combining must be disabled. Is this what caught you out?

— Reply to this email directly or view it on GitHubhttps://github.com/canton7/fuelphp-casset/issues/43#issuecomment-19922061 .