fix: prevent embedded styles to leak main document (#19274)

When using an exported a themed Flow web-component, Lumo style may leak the embedding document, causing invalid CSS rules to be applied. This change prevents applying Lumo global imports when the theme is applied to a web-component. Fixes #12704 Fixes #19265
vaadin · May 2, 2024 · f0890b4 · f0890b4
1 parent fa7d789
commit f0890b4
Show file tree

Hide file tree

Showing 12 changed files with 139 additions and 19 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java
@@ -96,6 +96,8 @@ abstract class AbstractUpdateImports implements Runnable {
             + " return !ae || ae.blur() || ae.focus() || true;\n" + "}";
     private static final String IMPORT_TEMPLATE = "import '%s';";
     private static final Pattern STARTING_DOT_SLASH = Pattern.compile("^\\./+");
+    private static final Pattern VAADIN_LUMO_GLOBAL_IMPORT = Pattern
+            .compile(".*@vaadin/vaadin-lumo-styles/.*-global.js.*");
     final Options options;
 
     private final UnaryOperator<String> themeToLocalPathConverter;
@@ -106,7 +108,8 @@ abstract class AbstractUpdateImports implements Runnable {
 
     private ClassFinder classFinder;
 
-    private final File generatedFlowImports;
+    final File generatedFlowImports;
+    final File generatedFlowWebComponentImports;
     private final File generatedFlowDefinitions;
     private File chunkFolder;
 
@@ -131,6 +134,10 @@ abstract class AbstractUpdateImports implements Runnable {
         generatedFlowDefinitions = new File(
                 generatedFlowImports.getParentFile(),
                 FrontendUtils.IMPORTS_D_TS_NAME);
+
+        generatedFlowWebComponentImports = FrontendUtils
+                .getFlowGeneratedWebComponentsImports(
+                        options.getFrontendDirectory());
         this.chunkFolder = new File(generatedFlowImports.getParentFile(),
                 "chunks");
 
@@ -146,6 +153,8 @@ public void run() {
 
         Map<File, List<String>> output = process(css, javascript);
         writeOutput(output);
+        writeWebComponentImports(
+                filterWebComponentImports(output.get(generatedFlowImports)));
 
         getLogger().debug("Imports and chunks update took {} ms.",
                 TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start));
@@ -207,6 +216,30 @@ protected void writeOutput(Map<File, List<String>> outputFiles) {
         }
     }
 
+    // Visible for test
+    List<String> filterWebComponentImports(List<String> lines) {
+        if (lines != null) {
+            // Exclude Lumo global imports for exported web-component
+            return lines.stream()
+                    .filter(VAADIN_LUMO_GLOBAL_IMPORT.asPredicate().negate())
+                    .collect(Collectors.toList());
+        }
+        return lines;
+    }
+
+    private void writeWebComponentImports(List<String> lines) {
+        if (lines != null) {
+            try {
+                generatedFilesSupport.writeIfChanged(
+                        generatedFlowWebComponentImports, lines);
+            } catch (IOException e) {
+                throw new IllegalStateException(
+                        "Failed to update the generated Flow imports for exported web component",
+                        e);
+            }
+        }
+    }
+
     /**
      * Processes what the scanner found and produces a set of files to write to
      * the generated folder.

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java
@@ -178,6 +178,12 @@ public class FrontendUtils {
      */
     public static final String IMPORTS_D_TS_NAME = "generated-flow-imports.d.ts";
 
+    /**
+     * Name of the file that contains application imports, javascript, theme and
+     * style annotations used when embedding Flow as web-component.
+     */
+    public static final String IMPORTS_WEB_COMPONENT_NAME = "generated-flow-webcomponent-imports.js";
+
     public static final String THEME_IMPORTS_D_TS_NAME = "theme.d.ts";
     public static final String THEME_IMPORTS_NAME = "theme.js";
 
@@ -1221,6 +1227,21 @@ public static File getFlowGeneratedImports(File frontendFolder) {
         return new File(getFlowGeneratedFolder(frontendFolder), IMPORTS_NAME);
     }
 
+    /**
+     * Gets the location of the generated import file for exported web
+     * components.
+     *
+     * @param frontendFolder
+     *            the project frontend folder
+     * @return the location of the generated import JS file for exported web
+     *         components
+     */
+    public static File getFlowGeneratedWebComponentsImports(
+            File frontendFolder) {
+        return new File(getFlowGeneratedFolder(frontendFolder),
+                IMPORTS_WEB_COMPONENT_NAME);
+    }
+
     /**
      * Gets the folder where exported web components are generated.
      *

diff --git a/...rver/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java b/...rver/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java
@@ -52,8 +52,8 @@ public class TaskGenerateWebComponentBootstrap
     protected String getFileContent() {
         List<String> lines = new ArrayList<>();
 
-        lines.add(
-                "import 'Frontend/generated/flow/generated-flow-imports.js';");
+        lines.add("import 'Frontend/generated/flow/"
+                + FrontendUtils.IMPORTS_WEB_COMPONENT_NAME + "';");
         lines.add("import { init } from '" + FrontendUtils.JAR_RESOURCES_IMPORT
                 + "FlowClient.js';");
         lines.add("init();");

diff --git a/...er/src/main/java/com/vaadin/flow/server/frontend/TaskRemoveOldFrontendGeneratedFiles.java b/...er/src/main/java/com/vaadin/flow/server/frontend/TaskRemoveOldFrontendGeneratedFiles.java
@@ -16,6 +16,7 @@
 
 package com.vaadin.flow.server.frontend;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.FileVisitResult;
@@ -54,11 +55,13 @@ public class TaskRemoveOldFrontendGeneratedFiles implements FallibleCommand {
     private static final Logger LOGGER = LoggerFactory
             .getLogger(TaskRemoveOldFrontendGeneratedFiles.class);
     private final Path frontendGeneratedFolder;
+    private final File frontendFolder;
 
     private final Set<Path> existingFiles = new HashSet<>();
     private GeneratedFilesSupport generatedFilesSupport;
 
     public TaskRemoveOldFrontendGeneratedFiles(Options options) {
+        frontendFolder = options.getFrontendDirectory();
         frontendGeneratedFolder = options.getFrontendGeneratedFolder().toPath();
         if (frontendGeneratedFolder.toFile().exists()) {
             try (Stream<Path> files = Files.walk(frontendGeneratedFolder)) {
@@ -123,10 +126,13 @@ public FileVisitResult postVisitDirectory(Path dir,
 
     private Predicate<Path> isKnownUnhandledFile() {
         Path flowGeneratedImports = FrontendUtils
-                .getFlowGeneratedImports(
-                        frontendGeneratedFolder.getParent().toFile())
-                .toPath().toAbsolutePath();
+                .getFlowGeneratedImports(frontendFolder).toPath()
+                .toAbsolutePath();
+        Path flowGeneratedWebComponentImports = FrontendUtils
+                .getFlowGeneratedWebComponentsImports(frontendFolder).toPath()
+                .toAbsolutePath();
         return path -> path.equals(flowGeneratedImports)
+                || path.equals(flowGeneratedWebComponentImports)
                 || path.equals(flowGeneratedImports
                         .resolveSibling(FrontendUtils.IMPORTS_D_TS_NAME))
                 || path.getFileName().toString()

diff --git a/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js b/flow-server/src/main/resources/plugins/application-theme-plugin/theme-generator.js
@@ -118,7 +118,9 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
       imports.push(`import { ${lumoImport} } from '@vaadin/vaadin-lumo-styles/${lumoImport}.js';\n`);
       if (lumoImport === 'utility' || lumoImport === 'badge' || lumoImport === 'typography' || lumoImport === 'color') {
         // Inject into main document the same way as other Lumo styles are injected
-        imports.push(`import '@vaadin/vaadin-lumo-styles/${lumoImport}-global.js';\n`);
+        // Lumo imports go to the theme global imports file to prevent style leaks
+        // when the theme is applied to an embedded component
+        globalFileContent.push(`import '@vaadin/vaadin-lumo-styles/${lumoImport}-global.js';\n`);
       }
     });
 

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/AbstractUpdateImportsTest.java
@@ -31,6 +31,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -124,6 +125,7 @@ public static class FooCssImport2 extends Component {
     class UpdateImports extends AbstractUpdateImports {
 
         private Map<File, List<String>> output;
+        private List<String> webComponentImports;
 
         UpdateImports(FrontendDependenciesScanner scanner, Options options) {
             super(options, scanner);
@@ -134,6 +136,12 @@ protected void writeOutput(Map<File, List<String>> output) {
             this.output = output;
         }
 
+        @Override
+        List<String> filterWebComponentImports(List<String> lines) {
+            webComponentImports = super.filterWebComponentImports(lines);
+            return webComponentImports;
+        }
+
         public Map<File, List<String>> getOutput() {
             return output;
         }
@@ -398,13 +406,41 @@ public void generate_containsLumoThemeFiles() {
         updater.run();
 
         assertContainsImports(true, "@vaadin/vaadin-lumo-styles/color.js",
+                "@vaadin/vaadin-lumo-styles/color-global.js",
                 "@vaadin/vaadin-lumo-styles/typography.js",
+                "@vaadin/vaadin-lumo-styles/typography-global.js",
                 "@vaadin/vaadin-lumo-styles/sizing.js",
                 "@vaadin/vaadin-lumo-styles/spacing.js",
                 "@vaadin/vaadin-lumo-styles/style.js",
                 "@vaadin/vaadin-lumo-styles/icons.js");
     }
 
+    @Test
+    public void generate_embeddedImports_doNotContainLumoGlobalThemeFiles()
+            throws IOException {
+        updater.run();
+
+        List<String> flowImports = new ArrayList<>(
+                updater.getOutput().get(updater.generatedFlowImports));
+
+        Predicate<String> lumoGlobalsMatcher = Pattern
+                .compile("@vaadin/vaadin-lumo-styles/.*-global.js")
+                .asPredicate();
+        assertTrue(flowImports.stream().anyMatch(lumoGlobalsMatcher));
+
+        assertTrue(
+                "Import for web-components should not contain lumo global imports",
+                updater.webComponentImports.stream()
+                        .noneMatch(lumoGlobalsMatcher));
+
+        // Check that imports other than lumo globals are the same
+        flowImports.removeAll(updater.webComponentImports);
+        assertTrue(
+                "Flow and web-component imports must be the same, except for lumo globals",
+                flowImports.stream().allMatch(lumoGlobalsMatcher));
+
+    }
+
     @Test
     public void jsModulesOrderIsPreservedAnsAfterJsModules() {
         updater.run();

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeTestComponents.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeTestComponents.java
@@ -157,7 +157,9 @@ public static class MainView extends Component {
      * Lumo component theme class implementation.
      */
     @JsModule("@vaadin/vaadin-lumo-styles/color.js")
+    @JsModule("@vaadin/vaadin-lumo-styles/color-global.js")
     @JsModule("@vaadin/vaadin-lumo-styles/typography.js")
+    @JsModule("@vaadin/vaadin-lumo-styles/typography-global.js")
     @JsModule("@vaadin/vaadin-lumo-styles/sizing.js")
     @JsModule("@vaadin/vaadin-lumo-styles/spacing.js")
     @JsModule("@vaadin/vaadin-lumo-styles/style.js")

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdateTestUtil.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/NodeUpdateTestUtil.java
@@ -85,7 +85,9 @@ List<String> getExpectedImports() {
                 "@vaadin/vaadin-lumo-styles/icons.js",
                 "@vaadin/vaadin-lumo-styles/style.js",
                 "@vaadin/vaadin-lumo-styles/typography.js",
+                "@vaadin/vaadin-lumo-styles/typography-global.js",
                 "@vaadin/vaadin-lumo-styles/color.js",
+                "@vaadin/vaadin-lumo-styles/color-global.js",
                 "@vaadin/vaadin-lumo-styles/sizing.js",
                 "@vaadin/vaadin-date-picker/theme/lumo/vaadin-date-picker.js",
                 "@vaadin/vaadin-date-picker/src/vaadin-month-calendar.js",

diff --git a/.../src/test/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrapTest.java b/.../src/test/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrapTest.java
@@ -17,16 +17,15 @@
 
 import java.io.File;
 
+import com.vaadin.flow.di.Lookup;
+import com.vaadin.flow.server.ExecutionFailedException;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.Mockito;
 
-import com.vaadin.flow.di.Lookup;
-import com.vaadin.flow.server.ExecutionFailedException;
-
 import static com.vaadin.flow.server.frontend.FrontendUtils.DEFAULT_FRONTEND_DIR;
 
 public class TaskGenerateWebComponentBootstrapTest {
@@ -57,7 +56,7 @@ public void should_importGeneratedImports()
         taskGenerateWebComponentBootstrap.execute();
         String content = taskGenerateWebComponentBootstrap.getFileContent();
         Assert.assertTrue(content.contains("import 'Frontend/generated/flow/"
-                + FrontendUtils.IMPORTS_NAME + "'"));
+                + FrontendUtils.IMPORTS_WEB_COMPONENT_NAME + "'"));
     }
 
     @Test

diff --git a/...er/src/test/java/com/vaadin/flow/server/frontend/scanner/FullDependenciesScannerTest.java b/...er/src/test/java/com/vaadin/flow/server/frontend/scanner/FullDependenciesScannerTest.java
@@ -338,7 +338,7 @@ public void getModules_explcitTheme_returnAllModulesExcludingNotUsedTheme_getCla
                     return null;
                 }, null);
 
-        DepsTests.assertImportCount(26, scanner.getModules());
+        DepsTests.assertImportCount(28, scanner.getModules());
         List<String> modules = DepsTests.merge(scanner.getModules());
         assertJsModules(modules);
 

diff --git a/flow-tests/test-embedding/test-embedding-application-theme/src/main/webapp/index.html b/flow-tests/test-embedding/test-embedding-application-theme/src/main/webapp/index.html
@@ -6,6 +6,7 @@
 
 <body>
 
+  <h1>Lumo styles should not be applied</h1>
   <div class="internal" id="internal">Internal should not apply, this should be black</div>
   <div class="global" id="global">Document styles should apply, this should be blue</div>
 

diff --git a/...ication-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java b/...ication-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java
@@ -18,15 +18,15 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-import org.junit.Assert;
-import org.junit.Test;
-import org.openqa.selenium.By;
-import org.openqa.selenium.WebElement;
-
 import com.vaadin.flow.component.html.testbench.DivElement;
+import com.vaadin.flow.component.html.testbench.H1Element;
 import com.vaadin.flow.component.html.testbench.SpanElement;
 import com.vaadin.flow.testutil.ChromeBrowserTest;
 import com.vaadin.testbench.TestBenchElement;
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
 import static com.vaadin.flow.webcomponent.ThemedComponent.EMBEDDED_ID;
 import static com.vaadin.flow.webcomponent.ThemedComponent.HAND_ID;
@@ -78,8 +78,7 @@ public void applicationTheme_GlobalCss_isUsedOnlyInEmbeddedComponent() {
                 "rgba(0, 0, 255, 1)", $("*").id("global").getCssValue("color"));
         Assert.assertEquals(
                 "Theme style should not be applied outside embedded component",
-                "rgba(24, 39, 57, 0.94)",
-                $("*").id("internal").getCssValue("color"));
+                "rgba(0, 0, 0, 1)", $("*").id("internal").getCssValue("color"));
 
         getDriver().get(getRootURL() + "/themes/embedded-theme/img/bg.jpg");
         Assert.assertFalse("app-theme background file should be served",
@@ -228,4 +227,23 @@ public void multipleSameEmbedded_cssTargetingDocumentShouldOnlyAddElementsOneTim
                 2l, getCommandExecutor().executeScript(
                         "return window.Vaadin.theme.injectedGlobalCss.length"));
     }
+
+    @Test
+    public void lumoImports_doNotLeakEmbeddingPage() {
+        open();
+        checkLogsForErrors();
+
+        // Ensure embedded components are loaded before testing embedding page
+        validateEmbeddedComponent($("themed-component").id("first"), "first");
+        validateEmbeddedComponent($("themed-component").id("second"), "second");
+
+        final H1Element element = $(H1Element.class).waitForFirst();
+        Assert.assertFalse(
+                "Lumo styles (typography) should not have been applied to elements in embedding page",
+                element.getCssValue("font-family").contains("Roboto"));
+        Assert.assertEquals(
+                "Lumo styles (colors) should not have been applied to elements in embedding page",
+                "rgba(0, 0, 0, 1)", element.getCssValue("color"));
+
+    }
 }