Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#5 | font_url_aggregation-1975138-2.patch | 1012 bytes | timaholt |
#2 | font_url_aggregation-1975138.patch | 356 bytes | timaholt |
Comments
Comment #1
timaholt CreditAttribution: timaholt commentedRealized 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.
Comment #2
timaholt CreditAttribution: timaholt commentedPatch attached.
Comment #3
timaholt CreditAttribution: timaholt commentedPatch works for http but not for https. Rethinking this.
Comment #4
Ignigena CreditAttribution: Ignigena commentedThe 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!
Comment #5
timaholt CreditAttribution: timaholt commentedActually 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!
Comment #6
Ignigena CreditAttribution: Ignigena commentedYes, 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.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedBefore 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).Comment #8
timaholt CreditAttribution: timaholt commentedI filed a core bug here: http://drupal.org/node/1978794
Comment #9
dudenhofer CreditAttribution: dudenhofer commentedI 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.