gaelj / BlazorCodeMirror6

Blazor wrapper for CodeMirror 6
MIT License
20 stars 2 forks source link

Additional exports #144

Closed pm64 closed 8 months ago

pm64 commented 8 months ago

To make it easier for external JavaScript to interoperate with this library, I wonder if certain exports could be added?

For example, I'm currently adding the following to index.ts to expose EditorViews, markdownLanguage, and csvToMarkdownTable():

import { markdownLanguage } from "@codemirror/lang-markdown"
import { csvToMarkdownTable } from "./CmColumns"

export function GetCmView(id: string): EditorView {
    return CMInstances[id].view;
}
export function GetMarkdownLanguage() {
    return markdownLanguage;
}
export function GetCsvToMarkdownTable(text: string, separator: string, withHeaders: boolean): string {
    return csvToMarkdownTable(text, separator, withHeaders);
}

Background: these are needed to handle clipboard operations from a different thread.

I'm not suggesting you literally add the above code, but rather, possibly provide some mechanism that makes things like these accessible, if possible.

gaelj commented 8 months ago

Exposing csvToMdTable, sure, why not. The rest, I have a feeling we could be able to solve what you need withing the library.

Could you provide some more detail (code samples) about the problem you wish to solve with these exports ?

pm64 commented 8 months ago

Sure, I'm handling the scenario where a clipboard operation is initiated from another thread. For security reasons, that's a no-go in Windows. So, I just need to perform the actual clipboard access from the calling thread, but make everything else happen as though initiated from within the editor.

So, you wind up adding some additional JavaScript in an app-specific module. For example, here's "copy":

window.myapp.copy = async function (id) {
    try {
        const view = GetCmView(id);
        const text = view.state.sliceDoc(view.state.selection.main.from, view.state.selection.main.to);
        if (text === null || text === undefined || text === "") return;
        await window.myapp.textEditorReference.invokeMethodAsync('SetClipboardAsync', text);
        view.focus()
        return true
    } catch (err) {
        console.error('Failed to copy text: ', err);
        return false;
    }
}

Since we can't call navigator.clipboard.writeText(text) directly, I call back into C# to do the actual copy (SetClipboardAsync) instead. But as you can see, a reference to the CodeMirror view itself is needed to make this work properly -- only obtainable, as far as I can tell, via my GetCmView() or a similar new export.

But I'm frequently wrong -- maybe it's already possible to get that reference without exposing anything new?

pm64 commented 8 months ago

@gaelj, thanks for adding these exports! I noticed one was left out, markdownLanguage:

export function GetMarkdownLanguage() {
    return markdownLanguage;
}

..which is imported into index.ts from @codemirror/lang-markdown:

import { markdownLanguage } from "@codemirror/lang-markdown"

It's needed for handling paste externally..

        if (GetMarkdownLanguage().isActiveAt(view.state, view.state.selection.main.from) &&
            GetMarkdownLanguage().isActiveAt(view.state, view.state.selection.main.to))
            text = csvToMarkdownTable(text, "\t", true)

Did you exclude it because this reference is already easy to get somehow? Sorry, definitely need to sharpen my JavaScript skills.

Thanks again for the updates!

gaelj commented 8 months ago

I'll export my clipboard related functions, I think it's cleaner in this way.

pm64 commented 8 months ago

I already tried that, unfortunately it doesn't work. Because of a Windows security policy, when the clipboard operation (cut/copy/paste) is initiated from the MAUI app's UI thread, any JavaScript then executed asynchronously within the Blazor app is denied clipboard access. That's why the cut/copy/paste functions I use call back into C# to do the raw clipboard access.

What might work is exporting special versions of cut/copy/paste that take a delegate responsible for raw clipboard access as a parameter. This would also help ensure that internally-initiated and externally-initiated cut/copy/paste behavior are consistent. I'll test that, just to see if it's an option -- but if this is getting too exotic, we can always table it.

gaelj commented 8 months ago

I mean, you can use the exported clipboard functions in the js side of your project. See line 66 of index.ts in 0.7.1

pm64 commented 8 months ago

Hey @gaelj, yeah I've tried lots of "easier" ways to go about this, none have worked so far.

I tried to recreate my experience exporting copy/cut/paste and then calling via interop (as you're proposing) in order to document the issues I encountered on that journey. Basically I recall getting it to where the clipboard operation silently failed (again, presumably due to the weird Windows security restrictions around the threads initiating the action and performing the clipboard access).

Instead, when calling cut() I'm now getting Failed to copy text: DOMException: Document is not focused. (Remember, this action would normally be initiated by a button click in the MAUI app that is communicating with the Blazor app by invoking a command on a shared view model or similar mechanism).

This is despite some pretty serious attempts I'm making to focus the document prior to the operation.. here's the last "cut" implementation I tested:

import {
    getCmInstance, cut, copy, paste
} from "./_content/GaelJ.BlazorCodeMirror6/index.js"

window.myApp.cut = async function (id) {

    var inst = getCmInstance(id).view;

    const timer = setInterval(() => {
        inst.focus();
        if (inst.hasFocus) clearInterval(timer);
    }, 500);

    await cut(inst);
};

Even if I were to get past this surprise focusing obstacle, I'd still expect the clipboard operation to silently fail, as it did in my previous testing.

I know that my solution of repeating/adapting your copy/cut/paste code in a separate module, calling back into C# to do clipboard access, and requiring unusual exports like markdownLanguage to make it all work probably makes you want to choke me, which I totally understand.

Since there may not be a cleaner way to do this, I'd be fine with considering these particular additional exports "too specific" to my app and not useful for the community. In a perfect world I could use this library untouched going forward, but I'll inevitably need to maintain a slightly modded version to support my unconventional use cases anyway. Priority #1 is to maintain the integrity of the library for you and all users.

gaelj commented 8 months ago

Clipboard access is a very sensitive area due to passwords and other personal data often being exposed there in plain text. To prevent websites/apps (such as TikTok) from harvesting this kind of data, lots of security checks were added to make sure the clipboard operation was initiated voluntarily by a human. In Firefox, clipboard read is disabled completely and unless the user installs a browser extension, there is nothing we can do about it.

But these restrictions apply to clipboard reads. Writing isn't a problem - in browsers at least. So I'm a bit surprised you are having problems with the cut function.

Anyway, if using my cut function from JS doesn't work, I can't imagine how writing exactly the same code yourself, requiring exposure of stuff that should be kept wrapped inside the wrapper along the way, would give any different result. You are basically trying to defeat a strong armor that was purposely built to prevent abuse. Even if you do find a way around it, it will likely get killed in the future. Isn't there a "standard" way to access the clipboard in MAUI windows that you can adapt from ? you should be able to retrieve the selected text(s) with the SelectionChanged property of the wrapper.

In your last code snippet, I think the timer is not completed when await cut is reached. Did you try to move the cut() inside the timer callback ?

pm64 commented 8 months ago

@gaelj, you're going to murder me. I decided to start over from scratch, and in doing so have now realized that there was never actually an issue here. I'm able to execute any clipboard operation from any thread by simply using the command dispatcher.

I don't know what was wrong with my initial tests, but a misreading of the Microsoft documentation led me to believe I'd hit a security restriction, when there was obviously just a defect in my test.

Very sorry about this, I am beyond embarrassed. There was no reason to export csvToMarkdownTable, cut, copy, or paste. In my opinion, those exports should be removed.

There's also no reason currently to export getCmInstance, however I think it's likely that one may be useful in the future.

gaelj commented 8 months ago

No problem, I'm glad you sorted it out.