Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrustedTypes policy usage in viewLayer #108240

Closed
jrieken opened this issue Oct 7, 2020 · 2 comments
Closed

TrustedTypes policy usage in viewLayer #108240

jrieken opened this issue Oct 7, 2020 · 2 comments
Assignees
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Oct 7, 2020

Continues #106285 (comment)

I have done the following measurement with the file attached below (uses the standalone editor). tt_N is run N using trusted types, str_N is run N with what we have today. I have picked _finishRenderingNewLines since that's where all changes are and insertAdjacentHTML which is receiving a trusted type

_finishRenderingNewLines insertAdjacentHTML
tt_1 20 17
tt_2 16 12
tt_3 17 13
str_1 17 16
str_2 15 13
str_3 16 15

sample.txt
profiles.zip

The first run with trusted types seems have been an outlier but otherwise I don't see a difference between both.

@jrieken jrieken self-assigned this Oct 7, 2020
@jrieken jrieken added debt Code quality issues engineering VS Code - Build / issue tracking / etc. labels Oct 7, 2020
@jrieken jrieken added this to the October 2020 milestone Oct 7, 2020
@jrieken
Copy link
Member Author

jrieken commented Oct 7, 2020

Changes that I have done

diff --git a/src/vs/editor/browser/view/viewLayer.ts b/src/vs/editor/browser/view/viewLayer.ts
index eec48240c9..5ab9784cd4 100644
--- a/src/vs/editor/browser/view/viewLayer.ts
+++ b/src/vs/editor/browser/view/viewLayer.ts
@@ -369,6 +369,8 @@ interface IRendererContext<T extends IVisibleLine> {
 
 class ViewLayerRenderer<T extends IVisibleLine> {
 
+	private static _ttPolicy = window.trustedTypes?.createPolicy('editorViewLayer', { createHTML: value => value });
+
 	readonly domNode: HTMLElement;
 	readonly host: IVisibleLinesHost<T>;
 	readonly viewportData: ViewportData;
@@ -505,6 +507,9 @@ class ViewLayerRenderer<T extends IVisibleLine> {
 	}
 
 	private _finishRenderingNewLines(ctx: IRendererContext<T>, domNodeIsEmpty: boolean, newLinesHTML: string, wasNew: boolean[]): void {
+		if (ViewLayerRenderer._ttPolicy) {
+			newLinesHTML = ViewLayerRenderer._ttPolicy.createHTML(newLinesHTML) as unknown as string; // explains the ugly casts -> https://github.com/microsoft/vscode/issues/106396#issuecomment-692625393
+		}
 		const lastChild = <HTMLElement>this.domNode.lastChild;
 		if (domNodeIsEmpty || !lastChild) {
 			this.domNode.innerHTML = newLinesHTML;

@alexdima
Copy link
Member

alexdima commented Oct 7, 2020

👍 Looks good, I tried with more heavy files.

@jrieken jrieken closed this as completed Oct 7, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.
Projects
None yet
Development

No branches or pull requests

2 participants