Attempt to fix highlighting issue in monaco

[jjspace]

Description

This is an attempt to fix the highlighting issue shown below from the feedback thread comment

I can't reproduce this myself as it seems specific to Windows + Chrome. Some investigation has pointed to potentially the ligature setting affecting this so this PR is to test that. More investigation showed this was a problem with loading fonts after Monaco tried to measure them.

Issue number and link

part of #12857

Testing plan

• Run npm run dev from the sandcastle package or npm run build-sandcastle and npm start from the project root
• Open the editor and do some text selection
• Click single tokens so the highlight shows up for them
• Confirm all highlights and cursors appear where you'd expect
• Test with multiple font and ligature enabled options in the settings

These steps reproduced the issue for me even on Linux:

• Set settings to a larger font size to make it more obvious
• Set font to Droid Sans, Ligatures OFF
• REFRESH
• Click Cesium in the editor to show the highlight in the background
• Switch to Cascadia Code font, observe offset (however small it is)

Same works for me from
Cascadia -> Fira
Droid -> Cascadia
Jetbrains -> Cascadia

Toggling Ligatures on the selected font also usually fixed it for me

Example on Linux:

Author checklist

• I have submitted a Contributor License Agreement
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• I have added or updated unit tests to ensure consistent code coverage
• I have updated the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[github-actions]

Thank you for the pull request, @jjspace!

✅ We can confirm we have a CLA on file for you.

[javagl]

Disclaimer: It's surprisingly hard to systematically test this. The behavior seems to change randomly. Building and reloading (with cache disabled) sometimes seems to pick up modified code, but sometimes not, and sometimes changing the 'Settings' seems to have an effect, and sometimes not, and sometimes only after reloading the page (not once but) multiple times. The settings appear to be stored in cookies (?). Maybe it's necessary to systematically delete them at certain points in the test procedure...?

However, I tried it out, and it didn't resolve it for me.

My strong recommendation for resolving this issue is:

Remove that "Droid Sans Mono" option, remove the "Font ligatures" toggle, and call it a day.

(If somebody cares, the response should be "Pull requests welcome!". How many substantially different forms of a "monospaced font" are there, after all ... ?)

---

Rant:

I'm recommending that after zooming a bit into that (with another disclaimer: I'm not a CSS/React expert, and I proudly refuse to become one). Apparently, fontLigatures can be a boolean or a string. And I tried to find out what the apparent string values are for fontLigatures are, but didn't find anything. Zero. Nothing. I randomly tried inserting the "" empty string or the string "false" and "off" at different places, but it didn't have any effect. Why did I try "off"? Well, the CSS seems to contain
--vscode-editorCodeLens-fontFeatureSettings: "liga" off
or
font-feature-settings: "liga" 0, "calt" 0;
at different places, so... why not?

My sincere condolences to everybody who has to deal with ... things ... like this:

---

Back on topic: It looks like the Settings defines fontLigatures to be boolean, which might have to be adjusted to inlcude string for this change anyhow.

[jjspace]

I did some more digging into this this morning. I'm more confident now saying this is an async issue with the font's loading after Monaco tries to measure them as mentioned in this issue microsoft/monaco-editor#392. This was linked from a few other alignment related issues too.

I've updated the code to revert the original ligature change but also delay setting the monaco font until it's loaded. The font loading API is not perfect and there might still be a better way of doing this. However, I have confirmed the issue is gone on my windows machine using the CI branch. @javagl can you please re-test this?

Other solutions might be to try and pre-load the font files but I wanted to avoid loading unnecessary fonts that people aren't using.

[javagl]

I tried it out, and despite all evil efforts of switching the fonts and toggling the ligatures, I could no longer reproduce the issue.

So this seems to fix it.

(Given that this apparently was some "synchronization/race-condition" thingy, it's hard to say for sure, but it seems to be fixed).

---

I looked at the code change, and cannot say anything about that. But in the future, when someone asks "Why do IT projects often take so much longer and cost so much more than originally planned?", I can say: "This 👉".
(Back in the days, I used to set a font for a component with component.setFont(new Font("Monospaced", PLAIN, 10)); 👴 )

[ggetz]

For simplicity, could we instead just wait on document.fonts.ready and then call monaco.editor.remeasureFonts()?

[ggetz]

Per MDN:

Note that the font.load() returns a promise, so we could have handled the completion of font loading by chaining then afterwards. Using document.fonts.ready can be better in some circumstances, as it is only called when all fonts in the document have been resolved and layout is complete.

[jjspace]

@ggetz Unfortunately not. That promise is a one time thing and it always returned instantly when I tried. On top of that, that promise and the other onfontloaded event handlers only trigger when the font is loaded using the fonts API and do not seem to trigger when a font is loaded because of styling changes (like how monaco updates it). I tried other arrangements of this and none worked so I settled on this as the best option.

[ggetz]

Ah, bummer.

[jjspace]

Auto merge won't go until it has an approve. CI passed on the last commit so I'm going to just merge