This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8909-397de92d779b3d8cb235116609cb4fe76d0c7da7 in repository https://gitbox.apache.org/repos/asf/texera.git
commit b143a4fd72160f71ceb88fa80d790c5f8d146db6 Author: Ryan Zhang <[email protected]> AuthorDate: Wed Oct 7 19:36:18 2026 +0000 fix(notebook-migration): apply the jupyter panel's read-only setup before the notebook loads (#8909) ### What changes were proposed in this PR? The Jupyter panel's cells became editable as soon as a viewer clicked into one. Classic Notebook resets each editor's `readOnly` from `cell.is_editable()` on every focus and re-enables the keyboard manager at the same time, so setting `readOnly` once from `custom.js` did not hold. That setup also raced the notebook: `custom.js` loads asynchronously and usually misses `notebook_loaded`, and only worked because it was bound to `kernel_ready` too. This PR moves the setup into `custom-preload.js`, a hook Notebook 6.5 provides that `main.js` waits for before it builds the notebook. Overrides placed there are always in effect: * `Cell.prototype.is_editable` returns false, so Notebook itself keeps every editor read only, including after focus. * `KeyboardManager.prototype.enable` is a no op, and the manager is disabled once the notebook loads, so focusing a cell no longer brings back Shift+Enter, `d d` and the other shortcuts. * `KernelSelector.prototype._spec_not_found` continues without a kernel instead of opening the "Kernel not found" dialog. The panel never runs cells, so a notebook whose kernel is not installed opens normally. Python notebooks still match `python3` and start their kernel as before. * The markdown `unrender` override moves over unchanged. `custom.js` keeps only the Texera messaging. Its cell click handler is now delegated from `document` and bound immediately, rather than on `kernel_ready`, so clicking a cell to highlight its operator also works when no kernel starts. It stays in `custom.js` because it needs the origin `start-texera-jupyter.sh` substitutes. `jupyter.dockerfile` copies the new file, and the local dev README lists it among the files that need an image rebuild. ### Any related issues, documentation, discussions? Closes #8908 Parent-issue #4301 R support (#8009) depends on this: R notebooks open in the panel without a kernel, which hit both the dialog and the editable cells. ### How was this PR tested? These files have no automated tests. Tested by hand in the Jupyter container, with real keystrokes: * An R notebook (no kernel installed): no dialog. Typing, Enter, Backspace, Shift+Enter, `d d` and `b` change nothing. Double clicking a markdown heading leaves it rendered. R syntax highlighting is intact, and the cell click handler is bound. * A Python notebook: the kernel still starts, highlighting is unchanged, and an editor stays read only after focus (before this change it became editable). * `custom-preload.js` is served by the rebuilt image. A server that was already running keeps serving Notebook's default stub until restarted, since Notebook caches static file locations. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) --- bin/dockerfiles/jupyter.dockerfile | 1 + bin/local-dev/README.md | 6 +- .../src/main/resources/custom-preload.js | 51 ++++++++++++++++ .../src/main/resources/custom.js | 70 +++++----------------- 4 files changed, 69 insertions(+), 59 deletions(-) diff --git a/bin/dockerfiles/jupyter.dockerfile b/bin/dockerfiles/jupyter.dockerfile index 1f911c5de4..2d234c2a3e 100644 --- a/bin/dockerfiles/jupyter.dockerfile +++ b/bin/dockerfiles/jupyter.dockerfile @@ -29,6 +29,7 @@ FROM jupyter/base-notebook:notebook-6.5.4 # The customizations live with notebook-migration-service, which owns the Jupyter # integration. Paths are repo-root relative: the build context is the repo root. COPY notebook-migration-service/src/main/resources/custom.js /home/jovyan/.jupyter/custom/custom.js +COPY notebook-migration-service/src/main/resources/custom-preload.js /home/jovyan/.jupyter/custom/custom-preload.js COPY notebook-migration-service/src/main/resources/custom.css /home/jovyan/.jupyter/custom/custom.css COPY notebook-migration-service/src/main/resources/start-texera-jupyter.sh /usr/local/bin/start-texera-jupyter.sh diff --git a/bin/local-dev/README.md b/bin/local-dev/README.md index ab84a9fbf6..b9ccbfa10a 100644 --- a/bin/local-dev/README.md +++ b/bin/local-dev/README.md @@ -114,9 +114,9 @@ invocation. `jupyter` is the only managed service that runs from a Texera-built image instead of natively, so edits to its customizations under -`notebook-migration-service/src/main/resources/` (`custom.js`, `custom.css`, -`start-texera-jupyter.sh`) do nothing until the image is rebuilt. CI publishes it, but a -local edit needs a local build under the same tag: +`notebook-migration-service/src/main/resources/` (`custom.js`, `custom-preload.js`, +`custom.css`, `start-texera-jupyter.sh`) do nothing until the image is rebuilt. CI +publishes it, but a local edit needs a local build under the same tag: ```sh docker build -f bin/dockerfiles/jupyter.dockerfile -t ghcr.io/apache/texera-jupyter:latest . diff --git a/notebook-migration-service/src/main/resources/custom-preload.js b/notebook-migration-service/src/main/resources/custom-preload.js new file mode 100644 index 0000000000..2211fa3ddb --- /dev/null +++ b/notebook-migration-service/src/main/resources/custom-preload.js @@ -0,0 +1,51 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Setup that must be in place before the notebook loads. main.js waits for this +// module, while custom.js loads alongside the notebook and can miss its events. +define([ + "base/js/events", + "notebook/js/cell", + "notebook/js/keyboardmanager", + "notebook/js/kernelselector", + "notebook/js/textcell", +], function (events, cell, keyboardmanager, kernelselector, textcell) { + // The embedded notebook is a read-only reference view of the generated workflow. + // Notebook re-derives each editor's readOnly from is_editable() on every focus. + cell.Cell.prototype.is_editable = function () { + return false; + }; + + // Focusing an editor re-enables the keyboard manager, which would bring back + // shortcuts such as Shift+Enter to run a cell. + keyboardmanager.KeyboardManager.prototype.enable = function () {}; + events.on("notebook_loaded.Notebook", function () { + Jupyter.keyboard_manager.disable(); + }); + + // The panel never runs cells, so a notebook whose kernel is not installed (an R + // notebook, for example) opens without one instead of prompting for a kernel. + kernelselector.KernelSelector.prototype._spec_not_found = function () { + this.events.trigger("no_kernel.Kernel"); + }; + + // Keep markdown cells rendered: overriding unrender() stops a double-click (or + // Enter) from dropping a markdown cell into its editable source view. + textcell.MarkdownCell.prototype.unrender = function () {}; +}); diff --git a/notebook-migration-service/src/main/resources/custom.js b/notebook-migration-service/src/main/resources/custom.js index 672104dfc0..ad432c0659 100644 --- a/notebook-migration-service/src/main/resources/custom.js +++ b/notebook-migration-service/src/main/resources/custom.js @@ -23,62 +23,20 @@ // without editing this file. const TEXERA_ORIGIN = "__TEXERA_ORIGIN__"; -// Use Jupyter's event system to ensure the notebook is fully loaded -require(["base/js/events"], function (events) { - events.on("kernel_ready.Kernel", function () { - - // Attach click event listener to cells. kernel_ready.Kernel fires on every - // kernel (re)start, so remove any previously bound handler first to avoid - // stacking duplicate listeners that would post N messages per click. - $("#notebook-container").off("click", ".cell").on("click", ".cell", function (event) { - const cell = $(this); - const index = $(".cell").index(cell); - const cellContent = cell.find(".input_area").text(); - - // Get the UUID from the cell's metadata, or use "N/A" if it doesn't exist - const cellUUID = Jupyter.notebook.get_cell(index).metadata.uuid || 'N/A'; - - // Send a message to the parent window (Texera app) - window.parent.postMessage( - { action: "cellClicked", cellIndex: index, cellContent: cellContent, cellUUID: cellUUID }, - TEXERA_ORIGIN - ); - }); - }); -}); - -// Read-only viewing: the embedded notebook is a reference view of the generated -// workflow, not an editing surface. Make every code cell's editor read-only and -// disable Jupyter's keyboard shortcuts so cells cannot be edited or executed. -// (The chrome hidden via custom.css already removes the run/save/menu controls.) -require(["base/js/events"], function (events) { - function makeReadOnly() { - if (!window.Jupyter || !Jupyter.notebook) { - return; - } - // readOnly "true" blocks editing while still letting the - // editor take focus, so a viewer can select and copy the generated code. - Jupyter.notebook.get_cells().forEach(function (cell) { - if (cell.code_mirror) { - cell.code_mirror.setOption("readOnly", true); - } - }); - if (Jupyter.keyboard_manager) { - Jupyter.keyboard_manager.disable(); - } - } - - // notebook_loaded fires once cells (and their editors) exist; kernel_ready - // fires on every (re)start and re-applies in case the manager was re-enabled. - events.on("notebook_loaded.Notebook", makeReadOnly); - events.on("kernel_ready.Kernel", makeReadOnly); -}); - -// Keep markdown cells rendered: overriding unrender() stops a double-click (or -// Enter) from dropping a markdown cell into its editable source view. The -// prototype override applies to all existing and future markdown cells. -require(["notebook/js/textcell"], function (textcell) { - textcell.MarkdownCell.prototype.unrender = function () {}; +// Delegated from document, so it can be bound before the notebook or a kernel exists. +$(document).on("click", "#notebook-container .cell", function () { + const cell = $(this); + const index = $(".cell").index(cell); + const cellContent = cell.find(".input_area").text(); + + // Get the UUID from the cell's metadata, or use "N/A" if it doesn't exist + const cellUUID = Jupyter.notebook.get_cell(index).metadata.uuid || 'N/A'; + + // Send a message to the parent window (Texera app) + window.parent.postMessage( + { action: "cellClicked", cellIndex: index, cellContent: cellContent, cellUUID: cellUUID }, + TEXERA_ORIGIN + ); }); // Listen for messages from the Texera app (or parent window)
