From d249ce17d0c2610c35230233317891dc5f234b0e Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Sun, 19 Jul 2026 21:40:53 +0200 Subject: [PATCH 1/2] Batch rich HTML updates --- .../jabref/htmltonode/rich/RichHtmlView.java | 27 +++++++++++++++++-- .../jabref/htmltonode/RichHtmlViewTest.java | 3 ++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java b/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java index 474a9db..09e1118 100644 --- a/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java +++ b/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java @@ -30,14 +30,15 @@ public class RichHtmlView extends StackPane { private final StringProperty html = new SimpleStringProperty(this, "html", ""); private final ObjectProperty options = new SimpleObjectProperty<>(this, "options", HtmlRenderOptions.defaults()); + private boolean updatesAreBatched; /// Creates an empty view with [HtmlRenderOptions#defaults()]. public RichHtmlView() { setAlignment(Pos.TOP_LEFT); getStylesheets().add(HtmlToNode.stylesheet()); getChildren().add(area); - html.addListener((observable, oldValue, newValue) -> rerender()); - options.addListener((observable, oldValue, newValue) -> rerender()); + html.addListener((observable, oldValue, newValue) -> rerenderIfNeeded()); + options.addListener((observable, oldValue, newValue) -> rerenderIfNeeded()); rerender(); } @@ -58,6 +59,22 @@ public final void setHtml(@Nullable String newHtml) { html.set(newHtml == null ? "" : newHtml); } + /// Updates the HTML content and render options in one model rebuild. + /// + /// @param newHtml the HTML content to render; `null` is treated as empty + /// @param newOptions the rendering options; `null` restores [HtmlRenderOptions#defaults()] + public final void setHtml(@Nullable String newHtml, @Nullable HtmlRenderOptions newOptions) { + HtmlRenderOptions effectiveOptions = newOptions == null ? HtmlRenderOptions.defaults() : newOptions; + updatesAreBatched = true; + try { + html.set(newHtml == null ? "" : newHtml); + options.set(effectiveOptions); + } finally { + updatesAreBatched = false; + } + rerender(); + } + /// The rendering options of this view. Setting a new value re-renders. Never `null`. /// /// @return the options property @@ -138,4 +155,10 @@ private void rerender() { area.setModel(RichTextRenderer.buildModel(HtmlToNode.parse(getHtml(), renderOptions.baseUri()), renderOptions)); RichTextRenderer.configure(area, renderOptions); } + + private void rerenderIfNeeded() { + if (!updatesAreBatched) { + rerender(); + } + } } diff --git a/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java b/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java index d032877..c10d632 100644 --- a/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java +++ b/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java @@ -7,6 +7,7 @@ import javafx.application.Platform; import javafx.scene.Scene; +import org.jabref.htmltonode.HtmlRenderOptions; import org.jabref.htmltonode.rich.RichHtmlView; import org.junit.jupiter.api.BeforeAll; @@ -73,7 +74,7 @@ void viewRendersHtmlIntoRichTextArea() throws Exception { Platform.runLater(() -> { try { RichHtmlView view = new RichHtmlView(); - view.setHtml("Kopp, O.: Some Paper"); + view.setHtml("Kopp, O.: Some Paper", HtmlRenderOptions.defaults()); new Scene(view, 400, 300); view.applyCss(); view.getRichTextArea().applyCss(); From 80084f3d7e605745d383a7a85f0ded21bdfc512d Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sun, 19 Jul 2026 22:00:24 +0200 Subject: [PATCH 2/2] Address review of batched rich HTML updates - Test the batching itself instead of only the new overload's signature: count model rebuilds for the batched call (1) against the two separate setters (2). Verified to fail when the batching guard is removed. - Restore the single-argument setHtml call in viewRendersHtmlIntoRichTextArea, which had lost its coverage to the new overload. - Re-render inside the finally block so a throwing property listener cannot leave the rendered content out of sync with the updated properties. - Document the batching exception on the class javadoc, which still claimed an unconditional re-render on every property change. - Add the new public API to the changelog. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01SoJLfS17Nn4jCHYX1WxoTP --- CHANGELOG.md | 4 ++ .../jabref/htmltonode/rich/RichHtmlView.java | 9 ++-- .../jabref/htmltonode/RichHtmlViewTest.java | 42 ++++++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f8c8c1..7d49b4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `RichHtmlView#setHtml(String, HtmlRenderOptions)`, which updates the HTML and the render options with a single model rebuild instead of one per property. + ## [0.1.0] - 2026-07-14 ### Added diff --git a/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java b/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java index 09e1118..ab9b595 100644 --- a/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java +++ b/src/main/java/org/jabref/htmltonode/rich/RichHtmlView.java @@ -22,8 +22,9 @@ /// `org.jabref.htmltonode.HtmlView`, but with native text selection, caret navigation, and /// accessibility. The area scrolls itself; do not wrap this view in a `ScrollPane`. /// -/// Re-renders whenever [#htmlProperty()] or [#optionsProperty()] changes. Must be used on the -/// JavaFX application thread. Requires the `jfx.incubator.richtext` module at runtime. +/// Re-renders whenever [#htmlProperty()] or [#optionsProperty()] changes, except while +/// [#setHtml(String,HtmlRenderOptions)] updates both — that renders once, at the end. Must be used +/// on the JavaFX application thread. Requires the `jfx.incubator.richtext` module at runtime. public class RichHtmlView extends StackPane { private final RichTextArea area = new HtmlRichTextArea(); @@ -70,9 +71,11 @@ public final void setHtml(@Nullable String newHtml, @Nullable HtmlRenderOptions html.set(newHtml == null ? "" : newHtml); options.set(effectiveOptions); } finally { + // In the finally block so a throwing property listener cannot leave the rendered + // content out of sync with the already-updated properties. updatesAreBatched = false; + rerender(); } - rerender(); } /// The rendering options of this view. Setting a new value re-renders. Never `null`. diff --git a/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java b/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java index c10d632..48766c2 100644 --- a/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java +++ b/src/test/java/org/jabref/htmltonode/RichHtmlViewTest.java @@ -2,6 +2,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import javafx.application.Platform; @@ -64,6 +65,45 @@ void selectionFacadeReturnsSelectedText() throws Exception { assertEquals(java.util.Optional.of("First entry\nSecond one"), after.get()); } + @Test + void setHtmlWithOptionsRebuildsModelOnce() throws Exception { + AtomicReference error = new AtomicReference<>(); + AtomicInteger batchedRebuilds = new AtomicInteger(); + AtomicInteger separateRebuilds = new AtomicInteger(); + AtomicReference renderedText = new AtomicReference<>(); + CountDownLatch done = new CountDownLatch(1); + + Platform.runLater(() -> { + try { + HtmlRenderOptions newOptions = HtmlRenderOptions.defaults().withBaseFontSize(19.0); + + RichHtmlView batched = new RichHtmlView(); + batched.getRichTextArea().modelProperty() + .addListener((observable, oldValue, newValue) -> batchedRebuilds.incrementAndGet()); + batched.setHtml("

Updated

", newOptions); + renderedText.set(batched.getRichTextArea().getModel().getPlainText(0)); + + RichHtmlView separate = new RichHtmlView(); + separate.getRichTextArea().modelProperty() + .addListener((observable, oldValue, newValue) -> separateRebuilds.incrementAndGet()); + separate.setHtml("

Updated

"); + separate.setOptions(newOptions); + } catch (Throwable t) { + error.set(t); + } finally { + done.countDown(); + } + }); + + assertTrue(done.await(15, TimeUnit.SECONDS), "FX task timed out"); + if (error.get() != null) { + throw new AssertionError("FX task failed", error.get()); + } + assertEquals(1, batchedRebuilds.get(), "batched update should rebuild the model once"); + assertEquals(2, separateRebuilds.get(), "separate setters rebuild the model per property"); + assertEquals("Updated", renderedText.get()); + } + @Test void viewRendersHtmlIntoRichTextArea() throws Exception { AtomicReference error = new AtomicReference<>(); @@ -74,7 +114,7 @@ void viewRendersHtmlIntoRichTextArea() throws Exception { Platform.runLater(() -> { try { RichHtmlView view = new RichHtmlView(); - view.setHtml("Kopp, O.: Some Paper", HtmlRenderOptions.defaults()); + view.setHtml("Kopp, O.: Some Paper"); new Scene(view, 400, 300); view.applyCss(); view.getRichTextArea().applyCss();