Constructor `InlineCssTextArea(String text)` initializes area with 1 paragraph with newline characters rather than multiple paragraphs

[TurekBot]

Expected Behavior

The BoundsPopups should disappear when they go outside of the window.

The BoundsPopups should also disappear when the window isn't focused.

Actual Behavior

The BoundsPopups don't disappear when they move outside the window.

The BoundsPopups don't disappear when the window isn't focused.

Reproducible Demo

See PopupDemo.java in richtextfx-demos.

[]Demo

Environment info:

• RichTextFX Version: v0.8.2
• Operating System: Windows 7 Professional
• Java version: 1.8.0_161

[JordanMartinez]

Thanks for pointing that out. Do you want to submit a PR to fix it?

[TurekBot]

I'd need a hand to keep consistent with the ReactFX stuff (I've never used ReactFX).

The problem seems to lie in this snippet:

    Subscription caretPopupSub = EventStreams.combine(caretBounds, caretPopup.outsideViewportValues())
                .subscribe(tuple3 -> {
                    Optional<Bounds> opt = tuple3._1;
                    boolean showPopupWhenCaretOutside = tuple3._2;

                    if (opt.isPresent()) {
                        Bounds b = opt.get();
                        caretPopup.setX(b.getMaxX() + caretXOffset);
                        caretPopup.setY(b.getMaxY() + caretYOffset);

                        if (caretPopup.isHiddenTemporarily()) {
                            caretPopup.show(stage);
                            caretPopup.setHideTemporarily(false);
                        }

                    } else {
                        if (!showPopupWhenCaretOutside) {
                            caretPopup.hide();
                            caretPopup.setHideTemporarily(true);
                        }
                    }
                });

It seems that before (meaning before something broke the Demo), the Optional wouldn't be "present" if the BoundsPopup was outside of the area's bounds. That's no longer true.

caretBounds comes from org.fxmisc.richtext.TextEditingArea#getCaretBounds and its documentation says

Gets the bounds of the caret in the Screen's coordinate system or Optional#empty()
if caret is not visible in the viewport.

But that doesn't seem to hold true any more. Am I missing something?

[JordanMartinez]

This actually reveals a bigger bug....

After I looked at it more, something baffled me. We can only get the caret's bounds if the paragraph is rendered on the screen. So, if the popup adheres to the location where the caret should be if the rendered paragraph wasn't removed, then its paragraph is still being rendered (just not something we see due to the clipping of the underlying viewport).

To confirm this, I printed out the visible paragraphs each time they changed.
This is what should appear:

Par[; StyledSegment(segment=Hello popup!; style=)]
Par[; StyledSegment(segment=2        END; style=)]
Par[; StyledSegment(segment=3        END; style=)]
Par[; StyledSegment(segment=4        END; style=)]
etc...

Here's what actually appeared on my console:

Par[; StyledSegment(segment=Hello popup!
2        END
3        END
4        END
5        END
6        END
7        END
... the rest of the paragraphs ...
99        END
 style=)]

Thus, the entire document is being rendered as a very long paragraph with multiple newlines inside of it, not many paragraphs for each line.

[JordanMartinez]

This bug was introduced back in #590 and is probably the reason behind the performance issues in #659 and #627

[JordanMartinez]

False alarm. The issue lies with InlineCssTextArea. When I decoupled the style from its segment, I didn't also update the area's constructor to use replace(int, int, StyledDocument) rather than replace(0, 0, SEG), which in this case is String. The difference in type meant calling ReadOnlyStyledDocument.fromSeg(), which sees the text as one segment that should go in only one paragraph, not ReadOnlyStyledDocument.fromString(), which will break the text across multiple paragraphs based on when it encounters a newline character.

[JordanMartinez]

The workaround is to use the no-args constuctor and then call the correct replace method after initializing it, along with whatever else is in the constructor that takes a String parameter.