microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
39.18k stars 3.52k forks source link

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

Open mifopen opened 1 year ago

mifopen commented 1 year ago

Reproducible in vscode.dev or in 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

hediet commented 1 year ago

Thanks for reporting! Up for a PR? ;)