Remove restriction that external stylesheets have .css extension
westonruter opened this issue · 4 comments
Initially the inclusion of external styles were limited to those that were on the filesystem. But since #1083 we also fetch external styles over HTTP and then cache them. So now if a URL lacks the .css
extension (or it is not found on the filesystem even) we can just fallback to fetching the stylesheet over the network.
This issue was noticed on WordPress.com VIP where automatic CSS concatenation results in a stylesheet URL that does not have a static filename extension.
Another related issue we can solve here. I've seen this come up twice now (e.g. #1269 (comment)) where a theme author includes a Google Font with an @import
rule like:
@import 'https://fonts.googleapis.com/css?family=Merriweather:300|PT+Serif:400i|Open+Sans:800|Zilla+Slab:300,400,500|Montserrat:800|Muli:400&subset=cyrillic-ext,latin-ext,cyrillic,greek,greek-ext,vietnamese';
This causes a strange validation error message to be raised:
Skipped file which does not have an allowed file extension (//fonts.googleapis.com/css).
Using @import
is not a recommended way to load CSS. It should use wp_enqueue_style()
instead, similar to how Twenty Seventeen is doing. This will ensure that the stylesheet is loaded at the optimal time, and that WordPress can add the preconnect
link.
Nevertheless, the AMP plugin should prevent trying to fetch such font stylesheets server-side to include in the concatenated script because AMP allows them to be referenced externally. In addition, the font should be cached according to its Cache-Control
headers and not stored in a transient.
So when we come across such an @import
, we should actually dynamically create a link
element in the document with an href
pointing to the imported URL.
/cc @igkuz
👍
Moving To 'Ready For Merging'
If it's alright, I'm moving this to 'Ready For Merging,' as I don't think it needs functional testing. Feel free to move it back.