From acedaf899d156495c08a162dafbef2c9ee988954 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 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. --- packages/compiler/src/shadow_css.ts | 8 ++-- .../compiler/test/ng_module_resolver_spec.ts | 27 ++++++++--- packages/compiler/test/shadow_css_spec.ts | 45 ++++++++++++++++--- 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/packages/compiler/src/shadow_css.ts b/packages/compiler/src/shadow_css.ts index 15e805c00f3c9e..c7a85a62bbbffb 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,12 +365,12 @@ 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); @@ -792,7 +790,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 572890edad839a..1526da8b51599d 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 16ddc93d0f61ae..1f883b8f9bde99 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,40 @@ 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 handle document rules', () => {