Skip to content

codemirror: Fix crash caused by token highlight

Warren Gifford requested to merge fkling/35491-invalid-position into main

Created by: fkling

Fixes #35491 (closed), #35555 (closed)

Problem

In hindsight this bug is "obvious" but it was quite obscure first. The core problem is that the extension which is responsible for highlighting the currently hovered token holds onto the token object even if token doesn't exist anymore. Should the token refer to a position outside of the new/current document, an error is thrown (video from the issue):

https://user-images.githubusercontent.com/179026/169289368-6b24bad8-90fc-4960-a628-e8ec13af4cc5.mp4

This only happens when the user hovers over a token and then continues to edit the input without moving mouse: We only cleared the token when the mouse leaves the input.

Solution

There are two parts to the solution:

  1. Instead of storing the hovered token, I'm just storing the hover position and look up the token when the decoration is created. This prevents storing stale data in the extension. Even if the stored position is invalid, looking up a token at that position wouldn't yield any result and thus wouldn't create a decoration.
  2. The hover position is now cleared when the document changes, i.e. the token highlight is removed when the input changes. This replicates Monaco's behavior. I also made the necessary changes to also hide the tooltip information.

Note: The first commit implements an alternative version: Instead of hiding the highlight and tooltip, they would both update themselves when the input changes. While this is nice, I feel like removing the additional "clutter" while the user is typing feels a bit better overall.

New behavior:

https://user-images.githubusercontent.com/179026/169290627-42753306-c239-499f-9c33-0ecc0a0dafe4.mp4

Since I was working on tooltips anyway, I also fixed the how/when tooltips are triggered. It feels more correct now:

https://user-images.githubusercontent.com/179026/169290763-b557b233-a36f-460e-aa6e-2ad3b784dd28.mp4

Test plan

Go to search page, enter foo bar in the search input. Hover over bar with the mouse cursor and let go of the mouse. Start deleting bar from the input (with backspace). The highlight and the tootip should hide immediately and no error should be thrown.

App preview:

Check out the client app preview documentation to learn more.

Merge request reports