From abcd4bbfaa79969da46aed1aeaf79abe5bb05b50 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 2 May 2021 13:16:49 +0200 Subject: [PATCH] fix(compiler): preserve @page rules in encapsulated styles (#41915) Currently the compiler treats `@page` rules in the same way as `@media`, however that is incorrect and it results in invalid CSS, because `@page` allows style declarations at the root (e.g. `@page (margin: 50%) {}`) and it only allows a limited set of at-rules to be nested into it. Given these restrictions, we can't really encapsulate the styles since they apply at the document level when the user tries to print. These changes make it so that `@page` rules are preserved so that we don't break the user's CSS. More information: https://www.w3.org/TR/css-page-3 Fixes #26269. PR Close #41915 --- packages/compiler/src/shadow_css.ts | 22 ++++---- .../compiler/test/ng_module_resolver_spec.ts | 27 +++++++--- packages/compiler/test/shadow_css_spec.ts | 54 ++++++++++++++++--- 3 files changed, 79 insertions(+), 24 deletions(-) diff --git a/packages/compiler/src/shadow_css.ts b/packages/compiler/src/shadow_css.ts index 15e805c00f..0cbf8c8b14 100644 --- a/packages/compiler/src/shadow_css.ts +++ b/packages/compiler/src/shadow_css.ts @@ -135,8 +135,6 @@ export class ShadowCss { strictStyling: boolean = true; - constructor() {} - /* * Shim some cssText with the given selector. Returns cssText that can * be included in the document via WebComponents.ShadowCSS.addCssToDocument(css). @@ -367,15 +365,15 @@ export class ShadowCss { return processRules(cssText, (rule: CssRule) => { let selector = rule.selector; let content = rule.content; - if (rule.selector[0] != '@') { + if (rule.selector[0] !== '@') { selector = this._scopeSelector(rule.selector, scopeSelector, hostSelector, this.strictStyling); } else if ( rule.selector.startsWith('@media') || rule.selector.startsWith('@supports') || - rule.selector.startsWith('@page') || rule.selector.startsWith('@document')) { + rule.selector.startsWith('@document')) { content = this._scopeSelectors(rule.content, scopeSelector, hostSelector); - } else if (rule.selector.startsWith('@font-face')) { - content = this._stripScopingSelectors(rule.content, scopeSelector, hostSelector); + } else if (rule.selector.startsWith('@font-face') || rule.selector.startsWith('@page')) { + content = this._stripScopingSelectors(rule.content); } return new CssRule(selector, content); }); @@ -396,15 +394,17 @@ export class ShadowCss { * :host ::ng-deep { * import 'some/lib/containing/font-face'; * } + * + * Similar logic applies to `@page` rules which can contain a particular set of properties, + * as well as some specific at-rules. Since they can't be encapsulated, we have to strip + * any scoping selectors from them. For more information: https://www.w3.org/TR/css-page-3 * ``` */ - private _stripScopingSelectors(cssText: string, scopeSelector: string, hostSelector: string): - string { + private _stripScopingSelectors(cssText: string): string { return processRules(cssText, rule => { const selector = rule.selector.replace(_shadowDeepSelectors, ' ') .replace(_polyfillHostNoCombinatorRe, ' '); - const content = this._scopeSelectors(rule.content, scopeSelector, hostSelector); - return new CssRule(selector, content); + return new CssRule(selector, rule.content); }); } @@ -792,7 +792,7 @@ function combineHostContextSelectors(contextSelectors: string[], otherSelectors: * in-place. * @param multiples The number of times the current groups should appear. */ -export function repeatGroups(groups: string[][], multiples: number): void { +export function repeatGroups(groups: string[][], multiples: number): void { const length = groups.length; for (let i = 1; i < multiples; i++) { for (let j = 0; j < length; j++) { diff --git a/packages/compiler/test/ng_module_resolver_spec.ts b/packages/compiler/test/ng_module_resolver_spec.ts index 572890edad..1526da8b51 100644 --- a/packages/compiler/test/ng_module_resolver_spec.ts +++ b/packages/compiler/test/ng_module_resolver_spec.ts @@ -7,15 +7,28 @@ */ import {NgModuleResolver} from '@angular/compiler/src/ng_module_resolver'; -import {ɵstringify as stringify} from '@angular/core'; -import {NgModule} from '@angular/core/src/metadata'; +import {Component, Directive, Injectable, NgModule, ɵstringify as stringify} from '@angular/core'; import {JitReflector} from '@angular/platform-browser-dynamic/src/compiler_reflector'; -class SomeClass1 {} -class SomeClass2 {} -class SomeClass3 {} -class SomeClass4 {} -class SomeClass5 {} +@Directive() +class SomeClass1 { +} + +@NgModule() +class SomeClass2 { +} + +@NgModule() +class SomeClass3 { +} + +@Injectable() +class SomeClass4 { +} + +@Component({template: ''}) +class SomeClass5 { +} @NgModule({ declarations: [SomeClass1], diff --git a/packages/compiler/test/shadow_css_spec.ts b/packages/compiler/test/shadow_css_spec.ts index 16ddc93d0f..574815ec5c 100644 --- a/packages/compiler/test/shadow_css_spec.ts +++ b/packages/compiler/test/shadow_css_spec.ts @@ -14,8 +14,11 @@ import {normalizeCSS} from '@angular/platform-browser/testing/src/browser_util'; function s(css: string, contentAttr: string, hostAttr: string = '') { const shadowCss = new ShadowCss(); const shim = shadowCss.shimCssText(css, contentAttr, hostAttr); - const nlRegexp = /\n/g; - return normalizeCSS(shim.replace(nlRegexp, '')); + return normalize(shim); + } + + function normalize(value: string): string { + return normalizeCSS(value.replace(/\n/g, '')).trim(); } it('should handle empty string', () => { @@ -53,10 +56,49 @@ import {normalizeCSS} from '@angular/platform-browser/testing/src/browser_util'; expect(s(css, 'contenta')).toEqual(expected); }); - it('should handle page rules', () => { - const css = '@page {div {font-size:50px;}}'; - const expected = '@page {div[contenta] {font-size:50px;}}'; - expect(s(css, 'contenta')).toEqual(expected); + // @page rules use a special set of at-rules and selectors and they can't be scoped. + // See: https://www.w3.org/TR/css-page-3 + it('should preserve @page rules', () => { + const contentAttr = 'contenta'; + const css = ` + @page { + margin-right: 4in; + + @top-left { + content: "Hamlet"; + } + + @top-right { + content: "Page " counter(page); + } + } + + @page main { + margin-left: 4in; + } + + @page :left { + margin-left: 3cm; + margin-right: 4cm; + } + + @page :right { + margin-left: 4cm; + margin-right: 3cm; + } + `; + const result = s(css, contentAttr); + expect(result).toEqual(normalize(css)); + expect(result).not.toContain(contentAttr); + }); + + it('should strip ::ng-deep and :host from within @page rules', () => { + expect(s('@page { margin-right: 4in; }', 'contenta', 'h')) + .toEqual('@page { margin-right:4in;}'); + expect(s('@page { ::ng-deep @top-left { content: "Hamlet";}}', 'contenta', 'h')) + .toEqual('@page { @top-left { content:"Hamlet";}}'); + expect(s('@page { :host ::ng-deep @top-left { content:"Hamlet";}}', 'contenta', 'h')) + .toEqual('@page { @top-left { content:"Hamlet";}}'); }); it('should handle document rules', () => {