Skip to content

Commit 04b3dee

Browse files
committed
fix(css): when compiling CSS, leave absolute imports alone
Closes angular#4592
1 parent 6b00b60 commit 04b3dee

File tree

7 files changed

+68
-6
lines changed

7 files changed

+68
-6
lines changed

modules/angular2/src/core/compiler/style_url_resolver.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ function extractUrls(resolver: UrlResolver, baseUrl: string, cssText: string, fo
2424
string {
2525
return StringWrapper.replaceAllMapped(cssText, _cssImportRe, (m) => {
2626
var url = isPresent(m[1]) ? m[1] : m[2];
27+
var schemeMatch = RegExpWrapper.firstMatch(_urlWithSchemaRe, url);
28+
if (isPresent(schemeMatch) && schemeMatch[1] != 'package') {
29+
// Do not attempt to resolve non-package absolute URLs with URI scheme
30+
return m[0];
31+
}
2732
foundUrls.push(resolver.resolve(baseUrl, url));
2833
return '';
2934
});
@@ -50,3 +55,6 @@ var _cssUrlRe = /(url\()([^)]*)(\))/g;
5055
var _cssImportRe = /@import\s+(?:url\()?\s*(?:(?:['"]([^'"]*))|([^;\)\s]*))[^;]*;?/g;
5156
var _quoteRe = /['"]/g;
5257
var _dataUrlRe = /^['"]?data:/g;
58+
// TODO: can't use /^[^:/?#.]+:/g due to clang-format bug:
59+
// https://github.com/angular/angular/issues/4596
60+
var _urlWithSchemaRe = /^['"]?([a-zA-Z\-\+\.]+):/g;

modules/angular2/src/core/dom/emulated_css.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ List<EmulatedCssRule> emulateRules(Iterable<cssv.TreeNode> rules) {
2828
return new EmulatedCssStyleRule(node);
2929
} else if (node is cssv.MediaDirective) {
3030
return new EmulatedCssMedialRule(node);
31+
} else if (node is cssv.ImportDirective) {
32+
return new EmulatedCssImportRule(node);
3133
}
3234
})
3335
.where((r) => r != null)
@@ -100,3 +102,12 @@ class EmulatedMediaList {
100102
.map((q) => q.span.text).join(' and ');
101103
}
102104
}
105+
106+
/// Emulates [CSSImportRule](https://developer.mozilla.org/en-US/docs/Web/API/CSSImportRule)
107+
class EmulatedCssImportRule extends EmulatedCssRule {
108+
EmulatedCssImportRule(cssv.ImportDirective directive) {
109+
this
110+
..type = 3
111+
..cssText = '@${directive.span.text};';
112+
}
113+
}

modules/angular2/test/core/compiler/shadow_css_spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,5 +154,11 @@ export function main() {
154154
var css = s('x >>> y {}', 'a');
155155
expect(css).toEqual('x[a] y[a] {}');
156156
});
157+
158+
it('should pass through @import directives', () => {
159+
var styleStr = '@import url("https://fonts.googleapis.com/css?family=Roboto");';
160+
var css = s(styleStr, 'a');
161+
expect(css).toEqual(styleStr);
162+
});
157163
});
158164
}

modules/angular2/test/core/compiler/style_url_resolver_spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,26 @@ export function main() {
8888
.toEqual(['http://ng.io/print1.css', 'http://ng.io/print2.css']);
8989
});
9090

91+
it('should leave absolute non-package @import urls intact', () => {
92+
var css = `@import url('http://server.com/some.css');`;
93+
var styleWithImports = resolveStyleUrls(urlResolver, 'http://ng.io', css);
94+
expect(styleWithImports.style.trim()).toEqual(`@import url('http://server.com/some.css');`);
95+
expect(styleWithImports.styleUrls).toEqual([]);
96+
});
97+
98+
it('should resolve package @import urls', () => {
99+
var css = `@import url('package:a/b/some.css');`;
100+
var styleWithImports = resolveStyleUrls(new FakeUrlResolver(), 'http://ng.io', css);
101+
expect(styleWithImports.style.trim()).toEqual(``);
102+
expect(styleWithImports.styleUrls).toEqual(['fake_resolved_url']);
103+
});
104+
91105
});
92106
}
107+
108+
/// The real thing behaves differently between Dart and JS for package URIs.
109+
class FakeUrlResolver extends UrlResolver {
110+
constructor() { super(); }
111+
112+
resolve(baseUrl: string, url: string): string { return 'fake_resolved_url'; }
113+
}

modules_dart/transform/lib/src/transform/common/url_resolver.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,8 @@ Uri toAssetScheme(Uri absoluteUri) {
5959
return absoluteUri;
6060
}
6161
if (absoluteUri.scheme != 'package') {
62-
throw new FormatException(
63-
'Unsupported URI scheme "${absoluteUri.scheme}" encountered.',
64-
absoluteUri);
62+
// Pass through URIs with non-package scheme
63+
return absoluteUri;
6564
}
6665

6766
if (absoluteUri.pathSegments.length < 2) {

modules_dart/transform/test/transform/common/url_resolver_tests.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ void allTests() {
8888
.toThrowWith(anInstanceOf: FormatException);
8989
});
9090

91-
it('should throw for unsupported schemes', () {
92-
expect(() => toAssetScheme(Uri.parse('file:///angular2')))
93-
.toThrowWith(anInstanceOf: FormatException);
91+
it('should pass through unsupported schemes', () {
92+
var uri = 'http://server.com/style.css';
93+
expect('${toAssetScheme(Uri.parse(uri))}').toEqual(uri);
9494
});
9595

9696
it('should throw if passed a null uri', () {

modules_dart/transform/test/transform/stylesheet_compiler/all_tests.dart

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ const SIMPLE_CSS = '''
1616
}
1717
''';
1818

19+
const HTTP_IMPORT = 'https://fonts.googleapis.com/css?family=Roboto';
20+
const CSS_WITH_IMPORT = '@import url(${HTTP_IMPORT});';
21+
1922
main() {
2023
Html5LibDomAdapter.makeCurrent();
2124
allTests();
@@ -60,6 +63,20 @@ allTests() {
6063
expect(transform.outputs[1].id.toString())
6164
.toEqual('somepackage|lib/style.css.shim.dart');
6265
});
66+
67+
it('should compile stylesheets with imports', () async {
68+
var cssFile = new Asset.fromString(
69+
new AssetId('somepackage', 'lib/style.css'), CSS_WITH_IMPORT);
70+
var transform = new FakeTransform()..primaryInput = cssFile;
71+
await subject.apply(transform);
72+
expect(transform.outputs.length).toBe(2);
73+
expect(transform.outputs[0].id.toString())
74+
.toEqual('somepackage|lib/style.css.dart');
75+
expect(transform.outputs[1].id.toString())
76+
.toEqual('somepackage|lib/style.css.shim.dart');
77+
expect(await transform.outputs[0].readAsString()).toContain(HTTP_IMPORT);
78+
expect(await transform.outputs[1].readAsString()).toContain(HTTP_IMPORT);
79+
});
6380
}
6481

6582
@proxy

0 commit comments

Comments
 (0)