obsidian-rtl icon indicating copy to clipboard operation
obsidian-rtl copied to clipboard

The RTL Support plugin breaks markdown tables

Open Aspect004 opened this issue 1 year ago • 4 comments

I made sure that the problem is not with Obsidian itself, it's the RTL Support plugin.

Steps to reproduce

Step 1: Create a markdown table. Step 2: Add 2000+ words to individual cells or even to one cell.

Additional information

Here’s a video showing the problem exactly: Video showing RTL Support bug

Aspect004 avatar Jul 24 '24 22:07 Aspect004

I can confirm I have the same issue. It made me go insane and never expected it would be from this plugin! I hope it get fixed soon.

m-primo avatar Jul 28 '24 21:07 m-primo

Thanks, will get it fixed!

esm7 avatar Jul 29 '24 03:07 esm7

Thought it's worth dropping an update here, so you all may know that in spite of struggling with this quite a bit, I'm still puzzled about what causes this. Basically it seems like inserting CodeMirror decorations (which are the HTML attributes used to control the line directions) to tables that include long texts, causes a RangeError from within Obsidian code when stepping over empty cells:

Uncaught RangeError: Position 1 is out of range for changeset of length 0
    at e.mapPos (app.js:1:261503)
    at vt (app.js:1:298283)
    at e.compare (app.js:1:294667)
    at t.update (app.js:1:353789)
    at e.update (app.js:1:443717)
    at e.dispatchTransactions (app.js:1:440278)
    at e.dispatch (app.js:1:442266)
    at t.set (app.js:1:1394187)
    at new t (app.js:1:1418386)
    at t.editTableCell (app.js:1:1413221)

In spite of trying a few directions of investigation and workarounds, I'm still unsure why this happens. It might even turn out to be an Obsidian bug. I'll continue to investigate and possibly reach out to the Obsidian devs if needed.

esm7 avatar Aug 08 '24 19:08 esm7

Have you had any luck on that one ?

Limezy avatar Sep 27 '24 15:09 Limezy

Unfortunately not, I tried everything I could but it seems to be failing deep inside Obsidian code -- just somehow (I guess indirectly) caused by the addition of extra direction attributes. I contacted the Obsidian devs about it but received no reply yet :-/

esm7 avatar Oct 08 '24 06:10 esm7

It looks like the new Obsidian 1.7.4 has solved the issue ?

Limezy avatar Oct 10 '24 04:10 Limezy

For me it hasn't :(

esm7 avatar Oct 10 '24 06:10 esm7

Indeed, I was too quick testing and had a false joy...

Don't know if it helps the debugging but in fact edition works for one of my cells that has a <span lang=he type="paragraph" xxxxx </span> inside.

Limezy avatar Oct 10 '24 10:10 Limezy

Please, open a thread here, describe the problem and provide a minimal reproducible example if possible. Thanks.

whitenoisedev avatar Oct 18 '24 02:10 whitenoisedev

Just for reference, the thread is here.

esm7 avatar Nov 05 '24 16:11 esm7

I can confirm I have the same issue, Obsidian 1.7.7.

Bacham avatar Jan 11 '25 10:01 Bacham

The key problem is, from now on, in the live preview mode, each table cell will create its own EditorView when you click on it. Thus, it's probably conflicted with the decorations that have been built based on the root view.

app.js:1 Uncaught RangeError: Position 151 is out of range for changeset of length 2
    at e.mapPos (app.js:1:264441)
    at Ct (app.js:1:301303)
    at e.compare (app.js:1:297687)
    at t.update (app.js:1:358349)
    at e.update (app.js:1:456858)
    at e.dispatchTransactions (app.js:1:453296)
    at e.dispatch (app.js:1:455407)
    at t.set (app.js:1:1652995)
    at new t (app.js:1:1680491)
    at t.editTableCell (app.js:1:1675191)

As we can see, Position 151 is the start offset of the viewport, and length 2 is the doc length of the table cell view. Because, it happens when the first line of the root view is not within the viewport range, and the table cell only has two characters for instance.

kotaindah55 avatar Feb 08 '25 13:02 kotaindah55

You can use this.view.state.field(editorEditorField) to retrieve currently active EditorView, instead of using this.view.state.field(editorInfoField).editMode.cm. It includes the one in the table cell. You can still use editorInfoField to gather MarkdownView informations. And for me, it fix the crash.

Hope it can give some help...

kotaindah55 avatar Feb 08 '25 13:02 kotaindah55

@kotaindah55,

Would you mind expanding on the above? I tried replacing the instances of obtaining EditorView objects into -

const editorView = (view.editor as any).cm.state.field(editorEditorField);

But it did not seem to make any difference. You seem to refer to the usage of editorInfo inside the constructor EditorPlugin, but I'm unsure why that's the place to be changed, since this is already where a view was given to the editor plugin, and it seems too late down the chain. Doesn't this function already hold the active EditorView (that's what the view variable is about)? I'm confused, and if you got it fixed, I really want to know how.

esm7 avatar Feb 27 '25 12:02 esm7

Okay, I'll try explaining it.

The first thing that we have to know is that, as explained, each table cell will create its own EditorView instance when you try editing on it. Because of that, the table cell's EditorView will have its own extension, including the ViewPlugin you have registered via Obsidian's Plugin instance.

Therefore, the table cell doesn't share the same ViewPlugin with the root document. Or, in summary, we have two ViewPlugins at the same time, one belongs to the root document, and the other belongs to the table cell.

The second thing is editorInfoField gives the information about the Markdown editor that the root document associates with. Hence, using like this.view.state.field(editorInfoField).editMode.cm only gives us the EditorView of the active root document, not the table cell. It would be different if we use this.view.state.field(editorEditorField) directly, the field directly leads us to the currently active EditorView (can be either the one that belongs to the root document, or the others such as that belongs to the table cell). (Correcting to what I said above, it's true that accessing editorEditorField only makes a waste, if you can actually take the active view from view)


We need to look first into this code:

constructor(view: EditorView) {
    this.decorations = this.buildDecorations(view);
    this.rtlPlugin = rtlPlugin;
    this.view = view;
    const editorInfo = this.view.state.field(editorInfoField);
    // Checking for editorInfo.editMode because apparently editorInfo.editor which is needed later
    // is a getter which counts on this field to exist
    if (editorInfo && editorInfo instanceof MarkdownView && (editorInfo as any).editMode) {
        this.rtlPlugin.adjustDirectionToView(editorInfo, this);
    }
    this.rtlPlugin.handleIframeEditor(this.view.dom, this.view, editorInfo.file, this);
}

We'll see that this.view.state.field(editorInfoField) casts MarkdownView into the editorInfo, and then to be processed by adjustDirectionToView. We don't see any problem here, until the view parameter give the different EditorView instance (the one that belongs to the table cell in this case) from that's stored in editorInfo.editor.cm.

Let's say that this is the EditorPlugin belonging to the table cell (and then will be passed as editorPlugin variable). We'll step forward into the adjustDirectionToView method:

adjustDirectionToView(view: MarkdownView, editorPlugin?: EditorPlugin) {
    if (!view)
        return;
    const file = view?.file;
    const editor = view?.editor;
    const editorView = (editor as any)?.cm as EditorView;
    if (file && file.path && editorView) {
        const [requiredDirection, _usedDefault] = this.getRequiredFileDirection(file);
        this.setMarkdownViewDirection(view, editor, editorView, requiredDirection, editorPlugin);
    }
}

editorView above refers to the root document's EditorView, and, once again, the editorPlugin doesn't belong to that view.

Skip setMarkdownViewDirection to adjustEditorPlugin. And that's where the actually problem lies.

adjustEditorPlugin(editorView: EditorView, newDirection: Direction, editorPlugin?: EditorPlugin) {
    let dispatchUpdate = false;
    if (!editorPlugin) {
        editorPlugin = editorView.plugin(this.editorPlugin);
        dispatchUpdate = true;
    }
    if (editorPlugin) {
        editorPlugin.setDirection(newDirection, editorView);
        // If we're not inside the context of a specific EditorPlugin, we need to dispatch an update
        // so the editor is refreshed
        if (dispatchUpdate)
            editorView.dispatch();
    }
}

We knew who the editorPlugin belongs to, and knew what the editorView refers to, then editorPlugin.setDirection(newDirection, editorView) starts to build decorations with the wrong EditorView.

setDirection(direction: Direction, view: EditorView) {
    this.direction = direction;
    this.decorations = this.buildDecorations(view);
}

And then, stores that decorations to the editorPlugin.decorations. So, it could produce RangeError when the decorations' ranges exceed the length of the current view.state.doc, which is, in this case, the length of the characters in the table cell.

kotaindah55 avatar Feb 27 '25 17:02 kotaindah55

To solve this issue, I added similarity checking between view and editorInfo.editMode.cm in the EditorPlugin constructor. Because, if it's not a table cell view, it should be the same as the root document view. Otherwise, adjust the direction directly through this.setDirection.

constructor(view: EditorView) {
    this.decorations = this.buildDecorations(view);
    this.rtlPlugin = rtlPlugin;
    this.view = view;
    const editorInfo = this.view.state.field(editorInfoField);
    // Checking for editorInfo.editMode because apparently editorInfo.editor which is needed later
    // is a getter which counts on this field to exist
    if (editorInfo && editorInfo instanceof MarkdownView && (editorInfo as any).editMode) {
        // Checking that view is the same as the main document view
        if ((editorInfo as any).editMode.cm === view) {
            this.rtlPlugin.adjustDirectionToView(editorInfo, this);
        }
        // May be a nested EditorView found in the main view, such as an EditorView of the table cell
        else {
            // Retrieving current document direction
            const file = editorInfo?.file;
            if (file && file.path) {
                [this.direction] = this.rtlPlugin.getRequiredFileDirection(file);
            }
            // Adjust the direction of the nested view
            this.setDirection(this.direction, this.view);
        }
    }
    this.rtlPlugin.handleIframeEditor(this.view.dom, this.view, editorInfo.file, this);
}

But, if we switch the direction via the command or the button, the direction of the table cell view above won't be updated.

Therefore, I inserted the code in the adjustEditorPlugin method that check if there any of the EditorView in the table cell.

adjustEditorPlugin(editorView: EditorView, newDirection: Direction, editorPlugin?: EditorPlugin) {
    let dispatchUpdate = false;
    if (!editorPlugin) {
        editorPlugin = editorView.plugin(this.editorPlugin);
        dispatchUpdate = true;
    }
    if (editorPlugin) {
        editorPlugin.setDirection(newDirection, editorView);
        // If we're not inside the context of a specific EditorPlugin, we need to dispatch an update
        // so the editor is refreshed
        if (dispatchUpdate)
            editorView.dispatch();
    }
    let editor = this.app.workspace.activeEditor?.editor;
    let activeEditorView = (editor as any)?.activeCM as EditorView;
    // Check if there any active table cell's EditorView
    if (activeEditorView && (editor as any).inTableCell as boolean) {
        // Retrieve EditorPlugin from the table cell view
        let tableCellEditorPlugin = activeEditorView.plugin(this.editorPlugin);
        tableCellEditorPlugin.setDirection(newDirection, activeEditorView);
        // Update the table cell direction
        activeEditorView.dispatch();
    }
}

In this case, you may use Editor.inTableCell to check if the Editor.activeCM is an EditorView from the table cell

I hope this will solve the issue, and sorry for my bad explaining.

kotaindah55 avatar Feb 27 '25 18:02 kotaindah55