Can't create cell on Safari when a previous cell is focused

[rodrigues]

Environment

• Elixir & Erlang/OTP versions (elixir --version): 1.14.4 / 25
• Operating system: Mac OS Ventura
• How have you started Livebook (mix phx.server, livebook CLI, Docker, etc): livebook cli
• Livebook version (use git rev-parse HEAD if running with mix): 0.9.3
• Browsers that reproduce this bug (the more the merrier): Safari (worked fine in Chrome)
• Include what is logged in the browser console: Nothing logged
• Include what is logged to the server console: Nothing logged

Current behavior

If the focus is on a cell not visible anymore, due to scrolling, when clicking on + Code, instead of creating a new cell, it simply goes back to the focused cell and doesn't create a new cell.

Here's a recording of the problem:

Screen.Recording.2023-06-22.at.15.16.13.mov

In this video I'm reproducing the error with some empty cells, but I've got this error already a few times in more real notebooks 😅

Expected behavior

Expected to behave the same in Chrome, to create a new cell and refocus on this new cell 💪

Thanks!

[awerment]

Seeing this one as well, on basically the same environment. It does not seem to matter how Livebook is started, it occurs when running the app or via mix phx.server.

Some data points to hopefully help track this down:

• the upstream cell is not scrolled back to if it is unfocused before clicking the insert button
• it only happens if the cell has focus, but is not visible

[awerment]

Think I've found the reason and a fix, but can't fully understand or explain it.

Monaco seems to force the focus back to the TextAreaInput if it loses focus but its content did not change? 🤔

From textAreaInput.js, lines 584 - 595:

// If the focus is outside the textarea, browsers will try really hard to reveal the textarea.
// Here, we try to undo the browser's desperate reveal.
try {
    const scrollState = dom.saveParentsScrollTop(textArea);
    this.setIgnoreSelectionChangeTime('setSelectionRange');
    textArea.focus();
    textArea.setSelectionRange(selectionStart, selectionEnd);
    dom.restoreParentsScrollTop(textArea, scrollState);
}
catch (e) {
    // Sometimes IE throws when setting selection (e.g. textarea is off-DOM)
}

We only end up in this path, if insertMode is enabled. The fix is to disable it in the Livebook session.js hook's handleDocumentMouseDown():

if (event.target.closest(`[data-el-insert-buttons] button`)) {
  this.setInsertMode(false); // <--- added
  return;
}

[jonatanklosko]

@awerment thanks for pinpointing the relevant vscode code :)

We can't do the workaround you suggested, because it effectively annihilates the if. The reason we need it in the first place, is that if a markdown cell is focused and you mouse-down on "+ Code", we want to keep editor focus (which includes re-focusing the Monaco editor). Otherwise (that is, if we exit focus mode) the markdown preview would collapse, the page would shift and the mouse-up would happen outside the button, so the click would be ignored.

Hopefully we can address this upstream, I opened an issue microsoft/monaco-editor#4042 with a possible fix.

[awerment]

@jonatanklosko Thanks for looking into this and the explanation! So the "fix" would cause the same issue (clicks not registering because of the page shifting) in other cases, I see.