Using the latest Drupal core (7.22) and Shiny, the Open Sans font fails to load when CSS Aggregation is turned on. This seems to be due to a longstanding core bug that drops @import tags when the files aren't local when aggregation is turned on.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

timaholt’s picture

Status: Needs review » Active

Realized this was just because the path started with // instead of http:// The // was making Drupal think it was a local file, and the @import tag was dropped when it couldn't find it. Forcing a full http:// allows it to work, but breaks https:// sites.

timaholt’s picture

Status: Active » Needs review
FileSize
356 bytes

Patch attached.

timaholt’s picture

Status: Active » Needs work

Patch works for http but not for https. Rethinking this.

Ignigena’s picture

The easiest fix for this might be to just hardcode in the https value. It will ensure no insecure content warnings on sites with SSL whilst not breaking the CSS aggregation.

Don't know if this is the best practice and I know that SSL sometimes has a bit of additional negotiating overhead which can add to the total load time. But running a test on Pingdom Tools it seems like the difference is minimal:

HTTP version (NY): 44ms
HTTPS version (NY): 74ms

Otherwise we'll need to go the PHP route and embed the OpenSans CSS within the document head rather than using an @import statement within the CSS file.

If I'm making no sense feel free to call me out. It's the end of a long week!

timaholt’s picture

Status: Needs work » Needs review
FileSize
1012 bytes

Actually this patch takes a different approach, and loads the protocol agnostic css file via a drupal_add_css in the preprocess_html hook. This should work on both, and also it works with aggregation turned on!

Ignigena’s picture

Version: 7.x-1.1 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Yes, that would be the "proper" method :)

Tested on my installation and everything seems to apply correctly to the 7.x-1.x development branch. Changing this to RTBC.

Damien Tournoud’s picture

Before committing, let's add a comment explaining why we are doing that. Also, please open a core issue. The bug is in drupal_load_stylesheet_content() (that uses a regexp (!) to exclude external includes).

timaholt’s picture

I filed a core bug here: http://drupal.org/node/1978794

dudenhofer’s picture

Status: Reviewed & tested by the community » Fixed

I applied the patch, added the comment, and replicated the update in the scss file as well. This has been committed now, so I'm marking this as fixed.

Status: Fixed » Closed (fixed)

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