Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Apply global styles before inserting the DOM element #48027

Merged
auto-submit merged 2 commits into from
Nov 15, 2023
Merged

[web] Apply global styles before inserting the DOM element #48027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43c837e
[web] Apply global styles before inserting the DOM element
mdebbar Nov 13, 2023
5d73ea3
Merge branch 'main' into style_sheet
mdebbar Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 73 additions & 93 deletions lib/web_ui/lib/src/engine/view_embedder/style_manager.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,106 +78,86 @@ void applyGlobalCssRulesToSheet(
String cssSelectorPrefix = '',
required String defaultCssFont,
}) {
// TODO(web): use more efficient CSS selectors; descendant selectors are slow.
// More info: https://csswizardry.com/2011/09/writing-efficient-css-selectors

assert(styleElement.sheet != null);
final DomCSSStyleSheet sheet = styleElement.sheet! as DomCSSStyleSheet;

// Fixes #115216 by ensuring that our parameters only affect the flt-scene-host children.
sheet.insertRule('''
$cssSelectorPrefix ${DomManager.sceneHostTagName} {
font: $defaultCssFont;
}
''', sheet.cssRules.length);
styleElement.appendText(
Copy link
Member

@ditman ditman Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a violation of the CSP of the page. Have you tried to run an app with this change with the following CSP?

script-src 'nonce-YOUR_NONCE_VALUE' 'wasm-unsafe-eval';
font-src https://fonts.gstatic.com;
style-src 'nonce-YOUR_NONCE_VALUE';

(The CSP can be set with just a meta-tag MDN, it needs to be a one-liner I think)

((Remember to set the nonce values in your flutter.js initialization, as described here))

Copy link
Contributor Author

@mdebbar mdebbar Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh, didn't think about that. Thanks for bringing it up. I'll try it and see what happens.

Copy link
Contributor Author

@mdebbar mdebbar Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be working fine in a sample app that I tried (I had to add nonce in many places to get it working 😜).

@ditman were you concerned that Element.appendText(...) is not allowed with CSP? Now that I think about it, how is it different from the insertRule API? They both take an arbitrary string.

Copy link
Member

@ditman ditman Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that insertRule is inserting the rule in CSSOM (so there's no possibility of XSS, AFAIK), but insertText is appending stuff in the DOM, so I thought that either TrustedTypes or CSP wouldn't like it! (I guess it's more like textContent than innerHTML 😅)

Copy link
Contributor Author

@mdebbar mdebbar Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's more like textContent than innerHTML 😅

Exactly. If you look at appendText in dom.dart, it's basically creating a Text Node and appending it. (and then, I assume, the CSS parser will parse it the same way it parses the string passed to insertRule).

Copy link

@cedvdb cedvdb Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just passing by but I believe XSS is possible in css too

Copy link
Member

@ditman ditman Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cedvdb I this case I think this is all right, because the stylesheet that is being created and injected is signed with flutter's nonce.

// Fixes #115216 by ensuring that our parameters only affect the flt-scene-host children.
'$cssSelectorPrefix ${DomManager.sceneHostTagName} {'
' font: $defaultCssFont;'
'}'

// This undoes browser's default painting and layout attributes of range
// input, which is used in semantics.
'$cssSelectorPrefix flt-semantics input[type=range] {'
' appearance: none;'
' -webkit-appearance: none;'
' width: 100%;'
' position: absolute;'
' border: none;'
' top: 0;'
' right: 0;'
' bottom: 0;'
' left: 0;'
'}'

// The invisible semantic text field may have a visible cursor and selection
// highlight. The following 2 CSS rules force everything to be transparent.
'$cssSelectorPrefix input::selection {'
' background-color: transparent;'
'}'
'$cssSelectorPrefix textarea::selection {'
' background-color: transparent;'
'}'

'$cssSelectorPrefix flt-semantics input,'
'$cssSelectorPrefix flt-semantics textarea,'
'$cssSelectorPrefix flt-semantics [contentEditable="true"] {'
' caret-color: transparent;'
'}'

// Hide placeholder text
'$cssSelectorPrefix .flt-text-editing::placeholder {'
' opacity: 0;'
'}',
);

// By default on iOS, Safari would highlight the element that's being tapped
// on using gray background. This CSS rule disables that.
if (isSafari) {
sheet.insertRule('''
$cssSelectorPrefix * {
-webkit-tap-highlight-color: transparent;
}
''', sheet.cssRules.length);
styleElement.appendText(
'$cssSelectorPrefix * {'
' -webkit-tap-highlight-color: transparent;'
'}'

'$cssSelectorPrefix flt-semantics input[type=range]::-webkit-slider-thumb {'
' -webkit-appearance: none;'
'}'
);
}

if (isFirefox) {
// For firefox set line-height, otherwise text at same font-size will
// measure differently in ruler.
//
// - See: https://github.com/flutter/flutter/issues/44803
sheet.insertRule('''
$cssSelectorPrefix flt-paragraph,
$cssSelectorPrefix flt-span {
line-height: 100%;
}
''', sheet.cssRules.length);
}

// This undoes browser's default painting and layout attributes of range
// input, which is used in semantics.
sheet.insertRule('''
$cssSelectorPrefix flt-semantics input[type=range] {
appearance: none;
-webkit-appearance: none;
width: 100%;
position: absolute;
border: none;
top: 0;
right: 0;
bottom: 0;
left: 0;
}
''', sheet.cssRules.length);

if (isSafari) {
sheet.insertRule('''
$cssSelectorPrefix flt-semantics input[type=range]::-webkit-slider-thumb {
-webkit-appearance: none;
}
''', sheet.cssRules.length);
styleElement.appendText(
'$cssSelectorPrefix flt-paragraph,'
'$cssSelectorPrefix flt-span {'
' line-height: 100%;'
'}'
);
}

// The invisible semantic text field may have a visible cursor and selection
// highlight. The following 2 CSS rules force everything to be transparent.
sheet.insertRule('''
$cssSelectorPrefix input::selection {
background-color: transparent;
}
''', sheet.cssRules.length);
sheet.insertRule('''
$cssSelectorPrefix textarea::selection {
background-color: transparent;
}
''', sheet.cssRules.length);

sheet.insertRule('''
$cssSelectorPrefix flt-semantics input,
$cssSelectorPrefix flt-semantics textarea,
$cssSelectorPrefix flt-semantics [contentEditable="true"] {
caret-color: transparent;
}
''', sheet.cssRules.length);

// Hide placeholder text
sheet.insertRule('''
$cssSelectorPrefix .flt-text-editing::placeholder {
opacity: 0;
}
''', sheet.cssRules.length);

// This CSS makes the autofill overlay transparent in order to prevent it
// from overlaying on top of Flutter-rendered text inputs.
// See: https://github.com/flutter/flutter/issues/118337.
if (browserHasAutofillOverlay()) {
sheet.insertRule('''
$cssSelectorPrefix .transparentTextEditing:-webkit-autofill,
$cssSelectorPrefix .transparentTextEditing:-webkit-autofill:hover,
$cssSelectorPrefix .transparentTextEditing:-webkit-autofill:focus,
$cssSelectorPrefix .transparentTextEditing:-webkit-autofill:active {
opacity: 0 !important;
}
''', sheet.cssRules.length);
styleElement.appendText(
'$cssSelectorPrefix .transparentTextEditing:-webkit-autofill,'
'$cssSelectorPrefix .transparentTextEditing:-webkit-autofill:hover,'
'$cssSelectorPrefix .transparentTextEditing:-webkit-autofill:focus,'
'$cssSelectorPrefix .transparentTextEditing:-webkit-autofill:active {'
' opacity: 0 !important;'
'}'
);
}

// Removes password reveal icon for text inputs in Edge browsers.
Expand All @@ -189,22 +169,22 @@ void applyGlobalCssRulesToSheet(
// so the below will throw an exception (because only real Edge understands
// the ::-ms-reveal pseudo-selector).
try {
sheet.insertRule('''
$cssSelectorPrefix input::-ms-reveal {
display: none;
}
''', sheet.cssRules.length);
styleElement.appendText(
'$cssSelectorPrefix input::-ms-reveal {'
' display: none;'
'}'
);
} on DomException catch (e) {
// Browsers that don't understand ::-ms-reveal throw a DOMException
// of type SyntaxError.
domWindow.console.warn(e);
// Add a fake rule if our code failed because we're under testing
assert(() {
sheet.insertRule('''
$cssSelectorPrefix input.fallback-for-fakey-browser-in-ci {
display: none;
}
''', sheet.cssRules.length);
styleElement.appendText(
'$cssSelectorPrefix input.fallback-for-fakey-browser-in-ci {'
' display: none;'
'}'
);
return true;
}());
}
Expand Down
7 changes: 5 additions & 2 deletions lib/web_ui/test/engine/global_styles_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ void testMain() {

setUp(() {
styleElement = createDomHTMLStyleElement(null);
domDocument.body!.append(styleElement);
applyGlobalCssRulesToSheet(
styleElement,
defaultCssFont: _kDefaultCssFont,
Expand Down Expand Up @@ -101,6 +100,7 @@ bool hasCssRule(
required String selector,
required String declaration,
}) {
domDocument.body!.append(styleElement);
assert(styleElement.sheet != null);

// regexr.com/740ff
Expand All @@ -110,7 +110,10 @@ bool hasCssRule(
final DomCSSStyleSheet sheet = styleElement.sheet! as DomCSSStyleSheet;

// Check that the cssText of any rule matches the ruleLike RegExp.
return sheet.cssRules
final bool result = sheet.cssRules
.map((DomCSSRule rule) => rule.cssText)
.any((String rule) => ruleLike.hasMatch(rule));

styleElement.remove();
return result;
}