On Fri, 10 Jul 2026 19:11:28 GMT, Andy Goryachev <[email protected]> wrote:
>> # Support for Embedded Images, Text Highlights, Wavy Underlines >> >> ## Summary >> >> This PR adds new attributes to support embedded images, text highlights, and >> wavy underlines. >> >> The `RichEditorDemoApp` gains new context menus and enables drag and drop to >> showcase the new functionality. >> >> <img width="768" height="703" alt="Screenshot 2026-06-18 at 12 40 28" >> src="https://github.com/user-attachments/assets/678bdbac-6582-46ee-bd41-ea938df930bb"; >> /> >> >> >> >> ## Problem >> >> Among the feedback received after publishing the RichTextArea incubating >> module, a number of users complained about the need to support embedded >> images. Other users complained about difficulty to get the text highlighted >> or wavy underlined, and having these decorations controlled via CSS. >> >> >> ## Solution >> >> Support for images was intentionally missing in the initial release; this PR >> adds the feature. >> >> Adding highlights and underlines is a bit more complicated, for several >> reasons. One reason is that the existing highlighting mechanism supports >> mixing of arbitrary number of translucent colors - something that is pretty >> much impossible to do with CSS alone. >> >> A useful compromise is to offer a limited number of standard attributes that >> can be controlled via CSS: 5 highlights and 3 wavy underlines. >> >> >> ## Specification >> >> >> ### StyleAttributeMap Attributes >> >> New attributes: >> - EMBEDDED_IMAGE >> - TEXT_HIGHLIGHT_1 ... TEXT_HIGHLIGHT_5 >> - UNDERLINE_WAVY_1 ... UNDERLINE_WAVY_3 >> >> ### CSS Styling >> >> The styling of highlights and wavy underlines is determined by the >> stylesheet and can be changed by the user. >> >> The style selectors are: >> >> `.rich-text-area .text-highlight-1` ... `.rich-text-area .text-highlight-5` >> `.rich-text-area .underline-wavy-1` ... `.rich-text-area .underline-wavy-3` >> >> >> ### New Public APIs >> >> `EmbeddedImage` >> `FileListFormatHandler` >> `StyleAttribute.inlineNode()` >> `StyleAttribute.isInlineNode()` >> `StyledSegment.ofInlineNode()` >> `CellContext.RunDecor` >> `CellContext.decorateRun()` >> `RichTextArea.dropTargetProperty` >> `RichTextArea.clearDropTarget()` >> `RichTextArea.getStyleAttributeMap()` >> >> >> ### File Format Changes >> >> The data transfer format (also used by the demo app to save the .rich files) >> version string is changed, making the old files unreadable (no backward >> compatibility will be provided during incubation period). >> >> >> ### Breaking API Changes >> >> `DataFormatHandler.createStyledInput()` >> `StyledTextMod... > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > closing ) I left a couple inline comments / questions. modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/RichTextModel.java line 927: > 925: b.addSegment(text, a); > 926: } else { > 927: b.addInlineNode(im::createNode, a); What if an inline node also has a non-empty text string? It looks like this will silently drop all of the text. Will this cause problems with API that uses TextPos? Would it be better to split the segment or else throw an error in this case? modules/jfx.incubator.richtext/src/main/java/jfx/incubator/scene/control/richtext/model/StyleAttribute.java line 208: > 206: > 207: /// inline node style attribute > 208: private static final class InlineNodeStyleAttribute<X> extends > StyleAttribute<X> { This addition has exposed a pair of related problems with the implementation of `equals` vs `hashCode` in `StyleAttribute`. The specific bug is that `equals` does not include a check on the new `isInlineNode()` attribute, whereas `hashCode`, by virtue of the fact that it includes the hash of the concrete subclass in its computation, effectively does. So at a minimum, you need to add a test for `isInlineImage()` in equals. However, it also raises a more general problem: you should not include `getClass()` in the hash code unless it is also explicitly included in the check for `equals` or you risk breaking the equals / hashCode contract (as happened here). For many cases, the concrete type of the class should not matter when computing the hash, but if it does, then `equals` must check it. Having `equals` check the type one way (using the isXXXX() methods) and `hashCode` check it another (using the concrete class type) is fragile. ------------- PR Review: https://git.openjdk.org/jfx/pull/2196#pullrequestreview-4688866633 PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3574356233 PR Review Comment: https://git.openjdk.org/jfx/pull/2196#discussion_r3574264694
