microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.39k stars 29.32k forks source link

Cursor position cannot be changed via touch, if embedded in AppleWebKit on an iPhone whose user-agent does not include 'chrome' and 'safari' #212484

Open CookieeQuinn opened 6 months ago

CookieeQuinn commented 6 months ago

Does this issue occur when all extensions are disabled?: Yes

Steps to Reproduce: The issue can be reproduced as long as:

  1. Ensure to use the Monaco editor embedded in AppleWebKit on an iPhone, ensuring that the user agent does not contain the word "Macintosh" but includes "AppleWebKit".
  2. Make sure the user agent does not contain the word "safari" or "chrome".

When embedded in AppleWebKit on an iPhone, the usability of the Monaco editor is severely compromised. You can only focus into the editor, seeing the cursor blinking at the beginning of the first line, and cannot change the cursor to other positions in the rows and columns (which also means you cannot select text, etc.).

However, it works fine in Google Chrome, Safari, or on Mac and Android devices. I found the following related issues and fixed the problem by adding 'user-select: auto':

I'm raising this issue mainly to inquire whether the official team has considered adding the 'enable-user-select' class when the user-agent 'isWebkitWebView'. Could you please provide some insights into the reason behind this decision? Thank you very much! @alexdima

Here are some details about this issue:

By debugging the relevant source code (_actualDoHitTestWithCaretRangeFromPoint), I found that after the user touches the screen, when the editor performs hit testing, the range returned by the caretRangeFromPoint method is null. Therefore, the hit test ultimately returns null, and we cannot obtain information about the hit object.

This results in the inability to change the cursor position.

Upon further inspection of the source code, it was found that when running in an environment where the user agent contains the words 'Safari', 'Chrome', or 'Macintosh', it works fine.

// microsoft/vscode/src/vs/editor/browser/config/editorConfiguration.ts

function getExtraEditorClassName(): string {
    let extra = '';
    if (!browser.isSafari && !browser.isWebkitWebView) {
        // Use user-select: none in all browsers except Safari and native macOS WebView
        extra += 'no-user-select ';
    }
    if (browser.isSafari) {
        // See https://github.com/microsoft/vscode/issues/108822
        extra += 'no-minimap-shadow ';
        extra += 'enable-user-select ';
    }
    if (platform.isMacintosh) {
        extra += 'mac ';
    }
    return extra;
}

// microsoft/vscode/src/vs/base/browser/browser.ts

export const isFirefox = (userAgent.indexOf('Firefox') >= 0);
export const isWebKit = (userAgent.indexOf('AppleWebKit') >= 0);
export const isChrome = (userAgent.indexOf('Chrome') >= 0);
export const isSafari = (!isChrome && (userAgent.indexOf('Safari') >= 0));
export const isWebkitWebView = (!isChrome && !isSafari && isWebKit);
export const isElectron = (userAgent.indexOf('Electron/') >= 0);
export const isAndroid = (userAgent.indexOf('Android') >= 0);
alexdima commented 5 months ago

@CookieeQuinn IIRC the problem was that caretRangeFromPoint was not working when invoked over text which used user-select: none.

I wonder if we should change https://github.com/microsoft/vscode/blob/ef849e41eefd7ce2494a0446ba594569b6e20e22/src/vs/editor/browser/config/editorConfiguration.ts#L227-L235

to

    if (browser.isSafari || browser.isWebkitWebView) {
        // See https://github.com/microsoft/vscode/issues/108822
        extra += 'no-minimap-shadow ';
        extra += 'enable-user-select ';
    } else {
        // Use user-select: none in all browsers except Safari and native macOS WebView
        extra += 'no-user-select ';
    }

Would you be able to test this and potentially submit a PR?

CookieeQuinn commented 4 months ago

@alexdima Thank you very much for your suggestion! I apologize for just seeing your reply recently.

I tested the solution today and it is feasible, and I just submitted a PR.https://github.com/microsoft/vscode/pull/219819

Additionally, as this is my first time submitting a PR to VSCode, I reviewed the PR list and observed that most PRs appear to target the main branch. Therefore, I submitted the PR to the main branch. Please let me know if this is not standard practice, Thank you very much.

Also, are there any specific requirements for branch naming when submitting a PR?

Please let me know if you need any further modifications or additional details.

Look forward to your response! Have a great day! :)