Skip to content

Commit

Permalink
Revert "fix: prevent embedded styles to leak main document (#19221)"
Browse files Browse the repository at this point in the history
This reverts commit 31e2cb5
  • Loading branch information
mshabarov committed Apr 26, 2024
1 parent a3e367a commit ef58049
Show file tree
Hide file tree
Showing 12 changed files with 17 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ 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;
Expand All @@ -108,8 +106,7 @@ abstract class AbstractUpdateImports implements Runnable {

private ClassFinder classFinder;

final File generatedFlowImports;
final File generatedFlowWebComponentImports;
private final File generatedFlowImports;
private final File generatedFlowDefinitions;
private File chunkFolder;

Expand All @@ -134,11 +131,6 @@ abstract class AbstractUpdateImports implements Runnable {
generatedFlowDefinitions = new File(
generatedFlowImports.getParentFile(),
FrontendUtils.IMPORTS_D_TS_NAME);

generatedFlowWebComponentImports = new File(
FrontendUtils.getFlowGeneratedWebComponentsFolder(
options.getFrontendDirectory()),
FrontendUtils.IMPORTS_WEB_COMPONENT_NAME);
this.chunkFolder = new File(generatedFlowImports.getParentFile(),
"chunks");

Expand All @@ -154,8 +146,6 @@ 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));
Expand Down Expand Up @@ -217,29 +207,6 @@ 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", e);
}
}
}

/**
* Processes what the scanner found and produces a set of files to write to
* the generated folder.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,6 @@ 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";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,6 @@ static Set<String> getGeneratedModules(File frontendFolder) {
return FileUtils
.listFiles(webComponentsFolder, new String[] { "js" }, true)
.stream()
.filter(file -> !FrontendUtils.IMPORTS_WEB_COMPONENT_NAME
.equals(file.getName()))
.map(file -> unixPath
.apply(baseDir.relativize(file.toURI()).getPath()))
.collect(Collectors.toSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public class TaskGenerateWebComponentBootstrap
protected String getFileContent() {
List<String> lines = new ArrayList<>();

lines.add("import 'Frontend/generated/flow/web-components/"
+ FrontendUtils.IMPORTS_WEB_COMPONENT_NAME + "';");
lines.add(
"import 'Frontend/generated/flow/generated-flow-imports.js';");
lines.add("import { init } from '" + FrontendUtils.JAR_RESOURCES_IMPORT
+ "FlowClient.js';");
lines.add("init();");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {

themeFileContent += `let needsReloadOnChanges = false;\n`;
const imports = [];
const deferredImports = [];
const componentCssImports = [];
const globalFileContent = [];
const globalCssCode = [];
Expand Down Expand Up @@ -119,9 +118,7 @@ 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
// Defer import at runtime to prevent leaking document when theme is applied
// to an embedded component
deferredImports.push(`import('@vaadin/vaadin-lumo-styles/${lumoImport}-global.js');\n`);
imports.push(`import '@vaadin/vaadin-lumo-styles/${lumoImport}-global.js';\n`);
}
});

Expand Down Expand Up @@ -223,8 +220,6 @@ function writeThemeFiles(themeFolder, themeName, themeProperties, options) {
const removers = [];
if (target !== document) {
${shadowOnlyCss.join('')}
} else {
${deferredImports.join('\n')}
}
${parentTheme}
${globalCssCode.join('')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
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;
Expand Down Expand Up @@ -125,7 +124,6 @@ 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);
Expand All @@ -136,12 +134,6 @@ 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;
}
Expand Down Expand Up @@ -406,41 +398,13 @@ 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,7 @@ 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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ 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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@

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 {
Expand Down Expand Up @@ -55,9 +56,8 @@ public void should_importGeneratedImports()
throws ExecutionFailedException {
taskGenerateWebComponentBootstrap.execute();
String content = taskGenerateWebComponentBootstrap.getFileContent();
Assert.assertTrue(content
.contains("import 'Frontend/generated/flow/web-components/"
+ FrontendUtils.IMPORTS_WEB_COMPONENT_NAME + "'"));
Assert.assertTrue(content.contains("import 'Frontend/generated/flow/"
+ FrontendUtils.IMPORTS_NAME + "'"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ public void getModules_explcitTheme_returnAllModulesExcludingNotUsedTheme_getCla
return null;
}, null);

DepsTests.assertImportCount(28, scanner.getModules());
DepsTests.assertImportCount(26, scanner.getModules());
List<String> modules = DepsTests.merge(scanner.getModules());
assertJsModules(modules);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

<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>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@
import java.util.List;
import java.util.stream.Collectors;

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 com.vaadin.flow.component.html.testbench.DivElement;
import com.vaadin.flow.component.html.testbench.SpanElement;
import com.vaadin.flow.testutil.ChromeBrowserTest;
import com.vaadin.testbench.TestBenchElement;

import static com.vaadin.flow.webcomponent.ThemedComponent.EMBEDDED_ID;
import static com.vaadin.flow.webcomponent.ThemedComponent.HAND_ID;
import static com.vaadin.flow.webcomponent.ThemedComponent.MY_COMPONENT_ID;
Expand Down Expand Up @@ -78,7 +78,8 @@ 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(0, 0, 0, 1)", $("*").id("internal").getCssValue("color"));
"rgba(24, 39, 57, 0.94)",
$("*").id("internal").getCssValue("color"));

getDriver().get(getRootURL() + "/themes/embedded-theme/img/bg.jpg");
Assert.assertFalse("app-theme background file should be served",
Expand Down Expand Up @@ -227,23 +228,4 @@ 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"));

}
}

0 comments on commit ef58049

Please sign in to comment.