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

[Bug]: Drag drop stops working in secondary window if main window isn't visible

Open pjeby opened this issue 1 year ago • 6 comments

Describe the bug

If I maximize a kanban secondary window (e.g. by double-clicking the Obsidian title bar, drag and drop stops working in it.

Expected Behavior

It should not stop working.

Steps to reproduce

Using 1.3.12 and Obsidian 0.15.9, open a kanban in a new window (I'm right-clicking a link to one). Attempt drag-drop, it works. Maximize the window, it stops working and gets the error shown below (repeatedly).

Relevant errors (if available)

Uncaught TypeError: Cannot read properties of undefined (reading 'set')
    at Ju.registerObserverHandler (plugin:obsidian-kanban:63:3845)
    at _l.initNodes (plugin:obsidian-kanban:19:96063)
    at _l.pollForNodes (plugin:obsidian-kanban:19:95827)
    at eval (plugin:obsidian-kanban:19:95803)

Operating system

Windows

pjeby avatar Aug 01 '22 07:08 pjeby

Addendum: it keeps working so long as it does not completely cover a monitor - resizing manually has the same effect as maximizing.

The effect also persists across Obsidian restart if the workspace is saved with the maximized window.

pjeby avatar Aug 01 '22 07:08 pjeby

Also seeing this warning appearing:

plugin:obsidian-kanban:formatted:21433 Event emitted with no handler dragStart undefined
emit @ plugin:obsidian-kanban:formatted:21433
dragStart @ plugin:obsidian-kanban:formatted:14843
eval @ plugin:obsidian-kanban:formatted:15039
eval @ plugin:obsidian-kanban:formatted:10942
requestAnimationFrame (async)
i @ plugin:obsidian-kanban:formatted:10940

pjeby avatar Aug 01 '22 07:08 pjeby

Further detail: it's only maximizing on my main monitor that's the problem. Maximizing on either my left or right secondary monitor doesn't produce the effect.

pjeby avatar Aug 01 '22 07:08 pjeby

And even more detail: it's not maximizing or sizing to the monitor, it's hiding the main window that causes the problem. If I minimze the Obsidian main window, the same thing happens, even if the secondary window isn't maximized.

pjeby avatar Aug 01 '22 08:08 pjeby

So, per Licat on Discord this is an issue where using the global requestAnimationFrame defers all the calls while the window isn't visible at least in part. The correct window's RAF has to be used, or else it'll hang.

I tried a few tricks like sticking const requestAnimationFrame = (cb) => activeWindow.requestAnimationFrame(cb); at the top of the compiled main.js does work reasonably well to fix the problem, but things move out of the way of the dragged card very slowly when the main window isn't visible. I tried scoping the scrollManager's IntersectionObserver to the correct window, but that doesn't make much difference. I also tried making setTimeout and clearTimeout use the active window, but that didn't help either.

My guess at this point is that there is some sort of ipc lag between the secondary and main window that batch-updates when the main isn't visible -- Licat indicated they'd had some problems with this with forwarding mousemove events so perhaps that's related?

pjeby avatar Aug 02 '22 00:08 pjeby

So it turned out to be setTimeout -- all uses of window.setTimeout and clearTimeout need to use the active window's version. My "const at the top" trick wasn't addressing the explicit window.setTimeout calls throughout the Kanban plugin.

I currently have a local plugin I built to monkeypatch all four APIs on the primary window (set/clear timeout, plus RAF/CAF) and the combo results in decent performance. The relevant patches look like this:

        this.register(around(window as Window, {
            requestAnimationFrame (old) {
                return function requestAnimationFrame(callback) {
                    if (activeWindow === this || !activeWindow) return old.call(this, callback);
                    return activeWindow.requestAnimationFrame(callback);
                };
            },
            cancelAnimationFrame (old) {
                return function cancelAnimationFrame(id) {
                    if (activeWindow === this || !activeWindow) return old.call(this, id);
                    return activeWindow.cancelAnimationFrame(id);
                };
            },
            setTimeout(old) {
                return function setTimeout(...args) {
                    if (activeWindow === this || !activeWindow) return old.apply(this, args);
                    return activeWindow.setTimeout(...args);
                }
            },
            clearTimeout(old) {
                return function clearTimeout(id) {
                    if (activeWindow === this || !activeWindow) return old.call(this, id);
                    return activeWindow.clearTimeout(id);
                }
            },
        }));

This is a bit of a hack since there's no guarantee the active window can't change between req/cancel or set/clear, but it works fine for interactivity. You can alternately use the banner trick to handle req/cancel, and change all the window.setTimeout calls to plain setTimeout, and replace Obsidian debounce to be able to use the banner trick for them as well. (Banner trick = adding function defs to the build banner to inject pseudo-globals that mask the real globals.) Doing that would let you fix this without making global changes to the Obsidian runtime environment.

pjeby avatar Aug 02 '22 01:08 pjeby