Skip to content

Commit

Permalink
Bug 1808741 - Patch lit's styleMap to not set the style attribute on …
Browse files Browse the repository at this point in the history 
…first render r=mtigley

lit calls `element.setAttribute("style", "<our styles>")` on first render of an
element so that it will have the style property set in the markup if it is being
server-side rendered. We don't need that, and it causes CSP errors in
about:logins so this patches out the functionality.

lit/rfcs#13

Differential Revision: https://phabricator.services.mozilla.com/D165240
  • Loading branch information
@mstriemer
mstriemer committed Jan 9, 2023
1 parent 68c4f24 commit a610fd5
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 48 deletions.
41 changes: 34 additions & 7 deletions toolkit/content/vendor/lit/0001-disable-terser-step.patch
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,34 @@
From 1f0db62374e62965f521c3a44c31c947f702d53d Mon Sep 17 00:00:00 2001
From d4897981e63b8be81453088cd9ab3a6edaf8fcaf Mon Sep 17 00:00:00 2001
From: Mark Striemer <[email protected]>
Date: Wed, 16 Nov 2022 22:54:20 -0600
Subject: [PATCH 1/4] disable terser step
Subject: [PATCH 1/5] disable terser step

---
rollup-common.js | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
rollup-common.js | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/rollup-common.js b/rollup-common.js
index ebe4b406..6fcac21a 100644
index b2187328..88d222d1 100644
--- a/rollup-common.js
+++ b/rollup-common.js
@@ -5,7 +5,7 @@
*/

import summary from 'rollup-plugin-summary';
-import {terser} from 'rollup-plugin-terser';
+// import {terser} from 'rollup-plugin-terser';
import copy from 'rollup-plugin-copy';
import nodeResolve from '@rollup/plugin-node-resolve';
import sourcemaps from 'rollup-plugin-sourcemaps';
@@ -321,7 +321,7 @@ export function litProdConfig({
(name) => `console.log(window.${name});`
),
].join('\n');
- const nameCacheSeederTerserOptions = generateTerserOptions(nameCache);
+ // const nameCacheSeederTerserOptions = generateTerserOptions(nameCache);

const terserOptions = generateTerserOptions(
nameCache,
@@ -345,7 +345,7 @@ export function litProdConfig({
virtual({
[nameCacheSeederInfile]: nameCacheSeederContents,
Expand Down Expand Up @@ -38,7 +56,16 @@ index ebe4b406..6fcac21a 100644
summary({
showBrotliSize: true,
showGzippedSize: true,
@@ -533,7 +533,7 @@ const litMonoBundleConfig = ({
@@ -524,7 +524,7 @@ const litMonoBundleConfig = ({
file,
output,
name,
- terserOptions,
+ // terserOptions,
format = 'umd',
sourcemapPathTransform,
// eslint-disable-next-line no-undef
@@ -563,7 +563,7 @@ const litMonoBundleConfig = ({
// This plugin automatically composes the existing TypeScript -> raw JS
// sourcemap with the raw JS -> minified JS one that we're generating here.
sourcemaps(),
Expand All @@ -48,5 +75,5 @@ index ebe4b406..6fcac21a 100644
showBrotliSize: true,
showGzippedSize: true,
--
2.32.0 (Apple Git-132)
2.37.1 (Apple Git-137.1)

8 changes: 4 additions & 4 deletions toolkit/content/vendor/lit/0002-use-DOMParser-not-innerHTML.patch
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
From e2fb71a850fda2d1f02a19d41a3db1cd7cf8618d Mon Sep 17 00:00:00 2001
From 0d300a2e703fdcc575ce36dfd62f6c3a9887a3ff Mon Sep 17 00:00:00 2001
From: Mark Striemer <[email protected]>
Date: Wed, 16 Nov 2022 23:07:57 -0600
Subject: [PATCH 2/4] use DOMParser not innerHTML=
Subject: [PATCH 2/5] use DOMParser not innerHTML=

---
packages/lit-html/src/lit-html.ts | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/packages/lit-html/src/lit-html.ts b/packages/lit-html/src/lit-html.ts
index 51941fdf..13914ca7 100644
index 896d298b..994c24e2 100644
--- a/packages/lit-html/src/lit-html.ts
+++ b/packages/lit-html/src/lit-html.ts
@@ -14,6 +14,8 @@ const NODE_MODE = false;
Expand Down Expand Up @@ -39,5 +39,5 @@ index 51941fdf..13914ca7 100644
}

--
2.32.0 (Apple Git-132)
2.37.1 (Apple Git-137.1)

29 changes: 19 additions & 10 deletions toolkit/content/vendor/lit/0003-Disable-source-maps.patch
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
From 365df0820b125d5e6cf1a709c98e02f1a0783311 Mon Sep 17 00:00:00 2001
From 28ad29d3496d10193de7a52bd8e104e183f5df7c Mon Sep 17 00:00:00 2001
From: Mark Striemer <[email protected]>
Date: Tue, 22 Nov 2022 18:17:01 -0600
Subject: [PATCH 3/4] Disable source maps
Subject: [PATCH 3/5] Disable source maps

---
rollup-common.js | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
rollup-common.js | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/rollup-common.js b/rollup-common.js
index 6fcac21a..cbfd1e27 100644
index 88d222d1..3f9e13b2 100644
--- a/rollup-common.js
+++ b/rollup-common.js
@@ -8,7 +8,7 @@ import summary from 'rollup-plugin-summary';
import {terser} from 'rollup-plugin-terser';
// import {terser} from 'rollup-plugin-terser';
import copy from 'rollup-plugin-copy';
import nodeResolve from '@rollup/plugin-node-resolve';
-import sourcemaps from 'rollup-plugin-sourcemaps';
Expand Down Expand Up @@ -53,10 +53,19 @@ index 6fcac21a..cbfd1e27 100644
}),
- sourcemaps(),
+ // sourcemaps(),
// We want the Node build to be minified because:
// We want the production Node build to be minified because:
//
// 1. It should be very slightly faster, even in Node where bytes
@@ -504,7 +504,7 @@ const litMonoBundleConfig = ({
@@ -496,7 +496,7 @@ export function litProdConfig({
'const NODE_MODE = false': 'const NODE_MODE = true',
},
}),
- sourcemaps(),
+ // sourcemaps(),
summary({
showBrotliSize: true,
showGzippedSize: true,
@@ -534,7 +534,7 @@ const litMonoBundleConfig = ({
file: `${output || file}.js`,
format,
name,
Expand All @@ -65,7 +74,7 @@ index 6fcac21a..cbfd1e27 100644
sourcemapPathTransform,
},
plugins: [
@@ -532,7 +532,7 @@ const litMonoBundleConfig = ({
@@ -562,7 +562,7 @@ const litMonoBundleConfig = ({
}),
// This plugin automatically composes the existing TypeScript -> raw JS
// sourcemap with the raw JS -> minified JS one that we're generating here.
Expand All @@ -75,5 +84,5 @@ index 6fcac21a..cbfd1e27 100644
summary({
showBrotliSize: true,
--
2.32.0 (Apple Git-132)
2.37.1 (Apple Git-137.1)

6 changes: 3 additions & 3 deletions toolkit/content/vendor/lit/0004-Remove-the-bundled-warning.patch
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 05d4da4eb99355580884d9685152f66f9e7f8d5d Mon Sep 17 00:00:00 2001
From db47e472d6f2393c58c1691d21e9bbc753b3d9da Mon Sep 17 00:00:00 2001
From: Mark Striemer <[email protected]>
Date: Tue, 22 Nov 2022 18:20:46 -0600
Subject: [PATCH 4/4] Remove the bundled warning
Subject: [PATCH 4/5] Remove the bundled warning

---
packages/lit/src/index.all.ts | 8 --------
Expand All @@ -24,5 +24,5 @@ index 1f8e5499..2bccd703 100644
- );
-}
--
2.32.0 (Apple Git-132)
2.37.1 (Apple Git-137.1)

109 changes: 109 additions & 0 deletions toolkit/content/vendor/lit/0005-Bug-1808741-Do-not-set-style-attributes-when-using-s.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
From 5f0afd7c556938cbf2693acf31656e9bcc01a3a5 Mon Sep 17 00:00:00 2001
From: Mark Striemer <[email protected]>
Date: Wed, 14 Dec 2022 15:34:57 -0600
Subject: [PATCH 5/5] Bug 1808741 - Do not set style attributes when using
styleMap

---
packages/lit-html/src/directives/style-map.ts | 21 +---------
.../src/test/directives/style-map_test.ts | 38 +------------------
2 files changed, 3 insertions(+), 56 deletions(-)

diff --git a/packages/lit-html/src/directives/style-map.ts b/packages/lit-html/src/directives/style-map.ts
index 5a77c676..cd7df040 100644
--- a/packages/lit-html/src/directives/style-map.ts
+++ b/packages/lit-html/src/directives/style-map.ts
@@ -43,21 +43,8 @@ class StyleMapDirective extends Directive {

render(styleInfo: Readonly<StyleInfo>) {
return Object.keys(styleInfo).reduce((style, prop) => {
- const value = styleInfo[prop];
- if (value == null) {
- return style;
- }
- // Convert property names from camel-case to dash-case, i.e.:
- // `backgroundColor` -> `background-color`
- // Vendor-prefixed names need an extra `-` appended to front:
- // `webkitAppearance` -> `-webkit-appearance`
- // Exception is any property name containing a dash, including
- // custom properties; we assume these are already dash-cased i.e.:
- // `--my-button-color` --> `--my-button-color`
- prop = prop
- .replace(/(?:^(webkit|moz|ms|o)|)(?=[A-Z])/g, '-$&')
- .toLowerCase();
- return style + `${prop}:${value};`;
+ // Make sure we use `styleInfo` so the TypeScript checker is happy. This is really a no-op.
+ return style + prop.slice(0, 0);
}, '');
}

@@ -66,10 +53,6 @@ class StyleMapDirective extends Directive {

if (this._previousStyleProperties === undefined) {
this._previousStyleProperties = new Set();
- for (const name in styleInfo) {
- this._previousStyleProperties.add(name);
- }
- return this.render(styleInfo);
}

// Remove old properties that no longer exist in styleInfo
diff --git a/packages/lit-html/src/test/directives/style-map_test.ts b/packages/lit-html/src/test/directives/style-map_test.ts
index 6f607e88..913f8a9e 100644
--- a/packages/lit-html/src/test/directives/style-map_test.ts
+++ b/packages/lit-html/src/test/directives/style-map_test.ts
@@ -4,8 +4,7 @@
* SPDX-License-Identifier: BSD-3-Clause
*/

-import {AttributePart, html, render} from 'lit-html';
-import {directive} from 'lit-html/directive.js';
+import {html, render} from 'lit-html';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map.js';
import {assert} from '@esm-bundle/chai';

@@ -33,41 +32,6 @@ suite('styleMap', () => {
container = document.createElement('div');
});

- test('render() only properties', () => {
- // Get the StyleMapDirective class indirectly, since it's not exported
- const result = styleMap({});
- // This property needs to remain unminified.
- const StyleMapDirective = result['_$litDirective$'];
-
- // Extend StyleMapDirective so we can test its render() method
- class TestStyleMapDirective extends StyleMapDirective {
- override update(
- _part: AttributePart,
- [styleInfo]: Parameters<this['render']>
- ) {
- return this.render(styleInfo);
- }
- }
- const testStyleMap = directive(TestStyleMapDirective);
- render(
- html`<div
- style=${testStyleMap({
- color: 'red',
- backgroundColor: 'blue',
- webkitAppearance: 'none',
- ['padding-left']: '4px',
- })}
- ></div>`,
- container
- );
- const div = container.firstElementChild as HTMLDivElement;
- const style = div.style;
- assert.equal(style.color, 'red');
- assert.equal(style.backgroundColor, 'blue');
- assert.include(['none', undefined], style.webkitAppearance);
- assert.equal(style.paddingLeft, '4px');
- });
-
test('first render skips undefined properties', () => {
renderStyleMap({marginTop: undefined, marginBottom: null});
const el = container.firstElementChild as HTMLElement;
--
2.37.1 (Apple Git-137.1)

6 changes: 3 additions & 3 deletions toolkit/content/vendor/lit/moz.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ origin:
"Lit is a simple library for building fast, lightweight web components."
url: "https://github.com/lit/lit"
license: "BSD-3-Clause"
release: lit@2.4.1 (2022-11-03T15:20:49-07:00).
revision: lit@2.4.1
release: lit@2.5.0 (2022-12-14T16:42:02-06:00).
revision: lit@2.5.0


# Since this tracks the latest tag, it's possible that lit isn't the latest tag
Expand All @@ -22,7 +22,7 @@ vendoring:
source-hosting: github
tracking: tag
# lit/lit is a monorepo that publishes multiple packages. The tags for lit
# are formatted as "lit@2.4.1".
# are formatted as "lit@2.5.0".
# tag-prefix: 'lit@'

vendor-directory: toolkit/content/vendor/lit/lit
Expand Down
24 changes: 3 additions & 21 deletions toolkit/content/widgets/vendor/lit.all.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ ReactiveElement.shadowRootOptions = { mode: 'open' };
polyfillSupport$2 === null || polyfillSupport$2 === void 0 ? void 0 : polyfillSupport$2({ ReactiveElement });
// IMPORTANT: do not change the property name or the assignment expression.
// This line will be used in regexes to search for ReactiveElement usage.
((_d$1 = global$1.reactiveElementVersions) !== null && _d$1 !== void 0 ? _d$1 : (global$1.reactiveElementVersions = [])).push('1.4.2');
((_d$1 = global$1.reactiveElementVersions) !== null && _d$1 !== void 0 ? _d$1 : (global$1.reactiveElementVersions = [])).push('1.5.0');

/**
* @license
Expand Down Expand Up @@ -2094,7 +2094,7 @@ const polyfillSupport$1 = global.litHtmlPolyfillSupport;
polyfillSupport$1 === null || polyfillSupport$1 === void 0 ? void 0 : polyfillSupport$1(Template, ChildPart$1);
// IMPORTANT: do not change the property name or the assignment expression.
// This line will be used in regexes to search for lit-html usage.
((_d = global.litHtmlVersions) !== null && _d !== void 0 ? _d : (global.litHtmlVersions = [])).push('2.4.0');
((_d = global.litHtmlVersions) !== null && _d !== void 0 ? _d : (global.litHtmlVersions = [])).push('2.5.0');
/**
* Renders a value, usually a lit-html TemplateResult, to the container.
*
Expand Down Expand Up @@ -4026,31 +4026,13 @@ class StyleMapDirective extends Directive {
}
render(styleInfo) {
return Object.keys(styleInfo).reduce((style, prop) => {
const value = styleInfo[prop];
if (value == null) {
return style;
}
// Convert property names from camel-case to dash-case, i.e.:
// `backgroundColor` -> `background-color`
// Vendor-prefixed names need an extra `-` appended to front:
// `webkitAppearance` -> `-webkit-appearance`
// Exception is any property name containing a dash, including
// custom properties; we assume these are already dash-cased i.e.:
// `--my-button-color` --> `--my-button-color`
prop = prop
.replace(/(?:^(webkit|moz|ms|o)|)(?=[A-Z])/g, '-$&')
.toLowerCase();
return style + `${prop}:${value};`;
return style + prop.slice(0, 0);
}, '');
}
update(part, [styleInfo]) {
const { style } = part.element;
if (this._previousStyleProperties === undefined) {
this._previousStyleProperties = new Set();
for (const name in styleInfo) {
this._previousStyleProperties.add(name);
}
return this.render(styleInfo);
}
// Remove old properties that no longer exist in styleInfo
// We use forEach() instead of for-of so that re don't require down-level
Expand Down

0 comments on commit a610fd5

Please sign in to comment.