RFR: 8366201: RichTextArea: remove allowUndo parameter [v2]
Kevin Rushforth
kcr at openjdk.org
Tue Oct 28 22:35:26 UTC 2025
On Wed, 22 Oct 2025 23:19:40 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> Original user feedback (see https://mail.openjdk.org/pipermail/openjfx-discuss/2025-August/000267.html ) called for adding an `allowUndo` parameter to `applyStyle()` and `setStyle()` methods similarly to `replaceText()`.
>>
>> Upon further analysis, the `allowUndo` parameter was a mistake: allowing the application code to disable creating undo/redo entries messes up the internal undo/redo stack.
>> There is an internal need (`UndoableChange`), but it should not be exposed via public API.
>>
>> This PR also adds `isUndoRedoEnabled()` and `setUndoRedoEnabled()` to the `StyledTextModel`, as well as its forwarding aliases to `RichTextArea` to allow for the application to disable undo/redo temporarily, for example, when building a document from multiple segments.
>>
>> WARNING this is an incompatible change, permitted because of the incubator.
>>
>> There remains a possible issue with currently unlimited size of the undo/redo stack - perhaps we should limit its depth to maybe 100-200 entries, see https://bugs.openjdk.org/browse/JDK-8370447 .
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>
> undo redo enabled logic
The API changes look good. I left a couple comments on the API docs.
Now that undoRedo is state, you should add tests for the set operations, checking default state and setting it to both false and true on the control both with / without model. I also recommend adding tests of its operation when calling undo / redo with the attr set to both false and true.
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/CodeArea.java line 439:
> 437:
> 438: /**
> 439: * Replaces text in this CodeArea. This method creates an undo/redo entry.
It creates an undo/redo entry if the undoRedo attribute is `true`.
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/RichTextArea.java line 1865:
> 1863:
> 1864: /**
> 1865: * Replaces the specified range with the new input. This method creates an undo entry.
It creates an undo/redo entry if the undoRedo attribute is `true`.
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledTextModel.java line 624:
> 622: * @param end end text position
> 623: * @param text text string to insert
> 624: * @param allowUndo when true, creates an undo-redo entry
Do you need to note that this method creates an undo/redo entry (if the undoRedo attribute is true)?
modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyledTextModel.java line 656:
> 654: * <p>
> 655: * After the model applies the requested changes, an event is sent to all the registered listeners.
> 656: * This method creates an undo/redo entry.
It creates an undo/redo entry if the undoRedo attribute is true.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1941#pullrequestreview-3391009394
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2471187922
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2471200486
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2471208075
PR Review Comment: https://git.openjdk.org/jfx/pull/1941#discussion_r2471208416
More information about the openjfx-dev
mailing list