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

[Bug] Mouse click incorrectly sets the caret position #3217

Closed
1 of 2 tasks
janarvaez opened this issue Jul 27, 2022 · 33 comments · Fixed by microsoft/vscode#178795
Closed
1 of 2 tasks

[Bug] Mouse click incorrectly sets the caret position #3217

janarvaez opened this issue Jul 27, 2022 · 33 comments · Fixed by microsoft/vscode#178795
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-shadow-dom verified Verification succeeded
Milestone

Comments

@janarvaez
Copy link

janarvaez commented Jul 27, 2022

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 Code

const customElement = document.createElement('div');
customElement.style.height = '100%';
customElement.attachShadow({mode: 'open'});

const customContainer = document.createElement('div');
customContainer.style.height = '100%';
customElement.shadowRoot.append(customContainer);

document.getElementById('container').append(customElement);

const editorRef = monaco.editor.create(customContainer, {
	value: `let wrapper = document.createElement('span');
wrapper.setAttribute('class', 'wrapper');
let icon = document.createElement('span');
icon.setAttribute('class', 'icon');
icon.setAttribute('tabindex', 0);
let info = document.createElement('span');
info.setAttribute('class', 'info');`,
      language: 'javascript',
});

Reproduction Steps

No response

Actual (Problematic) Behavior

I created a MonacoEditor which has issues with highlighting text and setting the caret in the desired position when clicking with the mouse, and I can't figure out what's going on.

I added a few console logs as an example, and for instance, I get:

// log1
mouseX: 376
mouseColumn: 16

// log2
mouseX: 223
mouseColumn: 23

What could be interfering with the correct position of the caret in the editor? How could it be that a click further to the right return a lower column number?
The deviation is not consistent or proportional either. If I click in the same place, I do get it in the same position. But clicking in one place can have +2 columns wrong, in another to the right have about +6 columns wrong, and further to the right the caret can be even positioned -7 columns in the other direction (like in the example above).

Expected Behavior

Clicking in the editor correctly positions the caret

Additional Context

No response

UPDATE: I found out the issue is when using the editor inside a shadowDOM


Edit by @hediet: Check here for verification steps

@hediet
Copy link
Member

hediet commented Jul 27, 2022

Can you provide an example in the playground to reproduce your issue? Thanks!

@hediet hediet added the info-needed Issue requires more information from poster label Jul 27, 2022
@janarvaez
Copy link
Author

Hi @hediet ! Thanks for the quick reply. I managed to get something that has similar issues with minimum code. My case is not as drastic, so in the example the caret won't move from the beginning, in my actual code it does move but to a wrong position. You can notice that if you click and type, the internal position does get moved though. I am using StencilJS for my custom elements with shadow DOM, so it might work better in my code due to stencil adding polyfills and other stuff.
Hope this helps

@hediet
Copy link
Member

hediet commented Jul 27, 2022

I'm sorry, but we need an example in our playground to properly investigate this bug and to rule out that, for example, StencilJS polyfills cause this issue.

@janarvaez
Copy link
Author

If you paste the code I added above in the description into the javascript tab in the playground it will reproduce

@janarvaez
Copy link
Author

There are some references to problems with the caret in this other issue:
#619

@hediet hediet added bug Issue identified by VS Code Team member as probable bug editor-shadow-dom and removed info-needed Issue requires more information from poster labels Jul 28, 2022
@lucasMarioza
Copy link

@janarvaez Did you find a fix or workaround? I'm facing the same issue and using monaco inside a shadowDOM is a requirement for me

@janarvaez
Copy link
Author

@janarvaez Did you find a fix or workaround? I'm facing the same issue and using monaco inside a shadowDOM is a requirement for me

I have 2 workarounds.

  1. have the container outside the shadowRoot passed as children through a slot. This works fine since the slotted container is actually outside, but wrapped and styled inside the shadow.
  2. Wrap the whole editor inside an iframe, but depending on your setup, this could be harder to achieve (although isolating the editor in an iframe prevents other issues, like having multiple editors with different languages rendered at the same time).

@lucasMarioza
Copy link

The workaround 1 worked but I figured it out my actual problem. I had one external style from an ancestor component(outside the shadowDOM) that was setting font-variant: tabular-nums. For some reason if monaco was rendered outside the shadowDOM this style was not an issue, and if it was inside the shadowDOM not even remeasureFonts fixed the issue.

I suppose that somehow it is not considering styles coming from outside the shadowDOM during the font measurements, or something like that.

In my case removing this style was not a problem, so I sticked with that solution.

@hediet hediet closed this as not planned Won't fix, can't repro, duplicate, stale Mar 13, 2023
@janarvaez
Copy link
Author

Hello @hediet !
Thanks for looking into this.
Unfortunately, that doesn't solve the issue. I do have the css in my real project as I can properly render the full editor and it renders correctly, but the cursor position is still there. It is even worse when trying to select text, as the highlighted selection doesn't follow the cursor.
In the link you sent, even if you added the css, the problem is still there for me.
The only way to fix this right now is to remove the shadowRoot in the web-component, which is not great

@janarvaezkng
Copy link

Screen.Recording.2023-03-16.at.09.25.15.mov

Please look at this reproduction video

@sorinui
Copy link

sorinui commented Mar 16, 2023

I also stumbled into this. It worked fine in Microsoft Edge versions prior to 110.0.1587.69 (and older Chrome versions).
The culprit seems to be window.getComputedStyle(el, null).getPropertyValue('font'); returning an empty string now. See
https://github.com/microsoft/vscode/blob/4c53c189a195c78a305ccce19d47341b9bee9baa/src/vs/editor/browser/controller/mouseTarget.ts#LL1037C12-L1037C12
@hediet can we reopen this issue?

@hediet hediet reopened this Mar 16, 2023
@hediet
Copy link
Member

hediet commented Mar 16, 2023

I can reproduce the bug in edge, but not chrome.

@sorinui can you look into this?

@janarvaezkng
Copy link

🤔 I'm using Chrome 111.0.5563.64 on mac and the bug is there

@lucasMarioza
Copy link

Disabling font-feature-settings: "liga" 0, "calt" 0; in the dev tools fixed the bug for me, but I'm not sure it will have anyother side effects
image

@hediet
Copy link
Member

hediet commented Mar 20, 2023

@lucasMarioza thanks for sharing this!

Enabling/disabling this rule actually changes if window.getComputedStyle(el, null).getPropertyValue('font'); returns an empty string or not (as @sorinui pointed out)

This really looks like a bug in chrome to me.

As a workaround, you can disable the font-feature-settings in the editor options.

@jogibear9988
Copy link

Has someone already raised a chrome issue?
We have the same problem, but only on some computers

@jogibear9988
Copy link

I created a chrome issue (cause after switch to 111 it also happens on my computer): https://bugs.chromium.org/p/chromium/issues/detail?id=1426792

@hediet
Copy link
Member

hediet commented Mar 22, 2023

I suggest you create a minimal reproducible example for them (not using monaco editor), demonstrating the issue that getPropertyValue('font') returns an empty string.

@jogibear9988
Copy link

I suggest you create a minimal reproducible example for them (not using monaco editor), demonstrating the issue that getPropertyValue('font') returns an empty string.

is this surely the cause of the issue?

@jogibear9988
Copy link

duplicate: #3858

jogibear9988 added a commit to node-projects/web-component-designer that referenced this issue Mar 22, 2023
@hediet hediet self-assigned this Mar 24, 2023
@hediet hediet added this to the April 2023 milestone Mar 24, 2023
@tursunova
Copy link

We analysed how this issue behaves in Chrome, Firefox, and Safari, looks like for Firefox Monaco editor is not accessing the font property for cursor positioning, that's why it is working correctly in Firefox, details here.

@leadpipe
Copy link

FYI, we at Google are still seeing the same Chrome behavior, though in a much narrower set of circumstances, with v0.37.0 (and now also on 0.37.1). (The good news for us is that we've found a workaround for it. See the end of the note for details.
But you'll probably still want to figure out what's going on.)

It's all working fine now on Macs. But selection continues to be broken on Windows. Here's a synopsis of what we're seeing:

  • On Windows, using Chrome 111.0.5563.147 or 112.0.5615.50, at normal resolution, the font metrics used for selection are narrower than those used to display the text. So near the ends of lines, particularly longer ones, the last few characters appear to be cut off from the selection. In fact, everything is selected just fine, it's just that the selection doesn't line up with the display so it's no longer WYSIWYG, which makes it essentially unusable.
  • If you change your resolution to 90% or 110% in these browsers, selection lines up with display like normal.
  • We've had a report that Chrome 111.0.5563.64 has no problem at 100% resolution.
  • Most interesting, if you reload the page the selection is drawn correctly.

Here's an example at 100% resolution (in Chrome 112.0.5615.50 on Windows) showing the problem:
image

The same screen at 90%:
image

And at 110%:
image

The workaround we have discovered is to add the fontLigatures property to your IStandaloneEditorConstructionOptions:

const EDITOR_CONFIG: monaco.editor.IStandaloneEditorConstructionOptions = {
  ...,
  fontLigatures: '',
};

With that option set, selection is drawn correctly even on Windows.

@jogibear9988
Copy link

@hediet
reopwn?

@drott
Copy link

drott commented Apr 18, 2023

The change in https://github.com/microsoft/vscode/pull/178795/files is not setting font-feature-settings for the measurements, which may explain the difference in measurement vs. displayed text.

@hediet hediet reopened this Apr 18, 2023
@hediet hediet modified the milestones: March 2023, April 2023 Apr 18, 2023
@hediet hediet modified the milestones: April 2023, May 2023 Apr 26, 2023
@hediet hediet modified the milestones: May 2023, June 2023 May 26, 2023
@TheDunco
Copy link

TheDunco commented Jun 2, 2023

I am experiencing this issue in my PayloadCMS instance which uses this package for their Code field. I'm running Chrome on MacOS. Is this the same issue or slightly different?

Screen.Recording.2023-06-02.at.1.58.12.PM.mov

@cdllqos
Copy link

cdllqos commented Jun 21, 2023

Situation as @TheDunco

@hediet
Copy link
Member

hediet commented Jun 29, 2023

I don't see this issue on Version 114.0.5735.199 (Official Build) (64-bit) on Windows (playground example), thus I assume this got fixed in Chrome.

chrome_AxwptXtdIC

The change in https://github.com/microsoft/vscode/pull/178795/files is not setting font-feature-settings for the measurements

This is true, but there does not seem to be a way to consider this setting when measuring the text with CanvasText.measureText(text: string), as Canvas only allows to set font, not font-feature-settings.

@hediet hediet closed this as completed Jun 29, 2023
@drott
Copy link

drott commented Jun 29, 2023

Re Chrome changes: We have at least not reverted or modified the font property serialisation behaviour change that we made in https://chromium-review.googlesource.com/c/chromium/src/+/4018412 and which played a key role in our analysis above, so I am not sure by what change this would have been affected on the Chromium side.

@hediet
Copy link
Member

hediet commented Jun 29, 2023

@drott thanks for letting us know! Once someone can reproduce with the latest chrome/monaco editor or vscode and can share reproducible steps, we'll reopen.

Off topic, but thanks so much for bringing back the "restart frame" feature in v8/Chrome! I use it daily and it just saves so much time!

@lramos15 lramos15 added the verified Verification succeeded label Jul 27, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug editor-shadow-dom verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.