Microsoft/monaco-editor

[Bug] Returning true from the callback registered with registerEditorOpener could lead to undesirable highlighting

Open

#4008 opened on Jun 6, 2023

View on GitHub
 (1 comment) (1 reaction) (0 assignees)JavaScript (14,836 stars) (1,283 forks)batch import
bughelp wantedtypescript

Description

Reproducible in vscode.dev or in VS Code Desktop?

  • Not reproducible in vscode.dev or VS Code Desktop

Reproducible in the monaco editor playground?

Monaco Editor Playground Link

https://microsoft.github.io/monaco-editor/playground.html?source=v0.38.0#XQAAAAJIBgAAAAAAAABBqQkHQ5NjdMjwa-jY7SIQ9S7DNlzs5W-mwj0fe1ZCDRFc9ws9XQE0SJE1jc2VKxhaLFIw9vEWSxW3yscw_NISC3wJUB_362YYT0xYObJ_rLmk-QNra3s1mo_V8GPD7nBqVzZWumY3n4A8YGBc0x4DCsAXNPz_e1SoQqExe8oFxNyeFh6ofEwLHb2Eq2nMEFMaIDMoswviivuAHyoei4urIiYxAVDvtClx7LWnsyUumLRPjKT9aHW7jiBUVDBnQkdhfrSOEbQ3asMcu0C67YTprPeXNLgMmzMptun3UKkeJm01CYlFX4R-AwrZP5m4n1VHEHKVO2tx_cvupRvUQKKmFLkuiLp3wE3juBV39X6GOSi4Hi6j5TAPB-U9O69lEDOsgSyvgZiUpoS0cCTK-ERxjT5DPqLiGMTg0D_hCHb-ek1e9BZOBo6-X1UAddZrZlyBzDRSSRrGAe0UP-6rqWgrOx9aN4ls_ZdN8qM7FoB0ssze31SyFNJGvTcUIMTr-W9lcb5fSPAHfp5HufWtRWuutRFAGyYgiRlUZB4i1DdojcOwc0uSVualxaoT1HxujXXfG0lBYG2zz8LUU0xEkPc-JmYw_luIk7VpOPGYd9g4IjQV28sI_N4JM89AKrvWWjNfMl0uXha3Fip7SIjHxmDH69__1CynyCntIBomTtri1GcxGn9h2YVVt9EIaw4QB3TViej_adpYTmPS1g8PhSItJXyjfJhCz6NtKdRSc7uDhyWVsMrPfIskE4X1UgB4SL64OrlaNx0g5EuBk2_4odk1t4zG_2Ly5xhXug0Axr5FVjeMICQ-CwRXSyfxSZ0HpVg4Bjn9eeHXgOiQM3gyOD4BuCUCA3RrSH6bhwgfQ1T16OqYHl6rc0Y1f__xP_EGGmKxvtagG11_DQC0-j8C023dPCBpHFuE0-mqla0DAnwOysh2eYpJuo-dDcnTnqH84QybH3wCnDGsoD7uyt3ht76KpTosSjRLw7uwJxpkOICwOWAW3-wFpIo72JUufq7tpdgxQG0svmX3Ea9AukS1hHrXyQeqmSaPm6clYwp3ez1IoyQrp5kFmh0UGA673sSjsxW2-LqED36uukPcFh7yOzQN-NroxiRUsiD-Vfaq

Monaco Editor Playground Code

// Add additional d.ts files to the JavaScript language service and change.
// Also change the default compilation options.
// The sample below shows how a class Facts is declared and introduced
// to the system and how the compiler is told to use ES6 (target=2).

// validation settings
monaco.languages.typescript.javascriptDefaults.setDiagnosticsOptions({
	noSemanticValidation: true,
	noSyntaxValidation: false,
});

// compiler options
monaco.languages.typescript.javascriptDefaults.setCompilerOptions({
	target: monaco.languages.typescript.ScriptTarget.ES2015,
	allowNonTsExtensions: true,
});

// extra libraries
var libSource = [
	"declare class Facts {",
	"    /**",
	"     * Returns the next fact",
	"     */",
	"    static next():string",
	"}",
].join("\n");
var libUri = "ts:filename/facts.d.ts";
monaco.languages.typescript.javascriptDefaults.addExtraLib(libSource, libUri);
// When resolving definitions and references, the editor will try to use created models.
// Creating a model for the library allows "peek definition/references" commands to work with the library.
monaco.editor.createModel(libSource, "typescript", monaco.Uri.parse(libUri));

var jsCode = [
	'"use strict";',
	"",
	"class Chuck {",
	"    greet() {",
	"        return Facts.next();",
	"    }",
	"}",
].join("\n");

monaco.editor.registerEditorOpener({
	async openCodeEditor(
		source,
		resource,
		selectionOrPosition,
	) {
		return true;
	}
});


monaco.editor.create(document.getElementById("container"), {
	value: jsCode,
	language: "javascript",
});

Reproduction Steps

  1. Open playground via attached link
  2. Cmd+click (ctrl+click) on the word "next" in the rendered editor

Actual (Problematic) Behavior

You should see some misplaced highlighting

Expected Behavior

No highlighting

Additional Context

I believe that the problem is here https://github.com/microsoft/vscode/blob/3da8c69a18ed0981f52e6f68ca6f8c1c19564179/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts#LL229C19-L229C19 There should be more strict condition as we should highlight only if we have new model open/visible in the editor

Contributor guide