Richtextfx: Random crashes in GenericStyleArea.visibleParToAllParIndex

[kszilagyi]

Expected Behavior

Shouldn't throw exception but return the index of the paragraph.

Actual Behavior

Sometimes when I call this method it reaches: throw new AssertionError("Unreachable code");
As much as I can tell the problem is that the element in the visibleParagraphs is the same as the one in the paragraphs but they are not the same reference therefore the search can't find it. I have no idea how is this possible.

Reproducible Demo

Unfortunately I only have this rarely...

Environment info:

• RichTextFX Version: 0.9.1
• Operating System: Ubuntu 18.04 but happened in 16.04 too.
• Java version: "1.8.0_181"

[kszilagyi]

Apparently resizing the window (sometimes?) fixes it.

[lhttjdr]

I write some blank paragraphs (only an ENTER each row).

• getVisibleParagraphs() will return an array with correct size (number of paragraphs).
• visibleParToAllParIndex(), firstVisibleParToAllParIndex() and lastVisibleParToAllParIndex() will throw out "Unreachable code" errors

I checked source code of visibleParToAllParIndex() in GenericStyledArea.java

        Paragraph<PS, SEG, S> visibleP = getVisibleParagraphs().get(visibleParIndex);
        for (int index = 0; index < getParagraphs().size(); index++) {
            if (getParagraph(index) == visibleP) {
                return index;
            }
        }

It is a simple comparison between results of getVisibleParagraphs() and getParagraphs().

Unfortunately, those two methods will give different results corresponding to the same blank paragraphs.

I use such a test case with 4 rows:

{Space}{Enter}
{Enter}
{Enter}
{Space}

, only the first and fourth row can be matched.

[Jugen]

@lhttjdr Thank you for the update to this issue.

Could you please provide code for a simple test case that throws out "Unreachable code" error.
I would really appreciate it, thanks.

[lhttjdr]

@Jugen Thank you for your great job at first.
I write some test cases for this problem and make a pull request.

[Jugen]

Ok, so after some research I've found that this happens because there is a discrepancy (based on ==) between the Paragraph objects returned by getParagraphs() and getVisibleParagraphs() in that the former contains the paragraphs submitted in an edit but the latter contains paragraphs merged between what was there before and whatever is there now.

So when a lookup is performed trying to find the exact object from the visible paragraph list in the list returned by getParagraphs, no match is found !

https://github.com/FXMisc/RichTextFX/blob/74efc7805014169f39eb2911fd01c562a73d31e1/richtextfx/src/main/java/org/fxmisc/richtext/GenericStyledArea.java#L823-L826

The solution then is to somehow make getParagraphs() to likewise contain the merged objects, and not necessarily the originally submitted edit.

[JFormDesigner]

@Jugen I have a fix for this issue in PR #795, which also fixes #788

Earlier today I had a look at #788 and later at this issue and noticed that both issues are related somehow to MaterializedListModification.trim(). It is in the stack of #788 and in Jugen's comment in this issue. So used the tests from @lhttjdr in PR #790 to see what's going on in trim() and why it may fail visibleParToAllParIndex.

The problem was that MaterializedListModification.trim(), which is invoked here, uses equals() to compare paragraphs and class Paragraph overrides equals() and returns true if paragraph text and styling is equal.

But this is not wanted because it keeps old outdated paragraphs and drops newly added paragraphs if the paragraph text and styling is equal.

The solution is to use a trim method that uses reference comparison instead of object comparison. Unfortunately there is no such method in ReactFX, so I copied the related methods to RichTextFX and modified them.

Below is some detailed information how I used println's to see whats going on:

I've added println's to the main constructor of class Paragraph:
~java
System.out.println("PARA "+System.identityHashCode(this));
~
To GenericStyledArea.visibleParToAllParIndex():
~java
System.out.println("vis "+System.identityHashCode(visibleP));
for (int index = 0; index < getParagraphs().size(); index++) {
System.out.println(" "+index+": "+System.identityHashCode(getParagraph(index)));
}
~

and to GenericEditableStyledDocumentBase$ParagraphList.observeInputs():
~java
for (...) {
p(mod);
mod = mod.trim();
p(mod);
...
}
~

Here is p(mod):
~java
private void p(MaterializedListModification> mod) {
System.out.println("MOD from "+mod.getFrom());
System.out.print(" added "+mod.getAddedSize()+":");
mod.getAdded().stream().forEach(p -> System.out.print(" "+System.identityHashCode(p)));
System.out.println();
System.out.print(" removed "+mod.getRemovedSize()+":");
mod.getRemoved().stream().forEach(p -> System.out.print(" "+System.identityHashCode(p)));
System.out.println();
}
~

When you run test get_first_visible_paragraph_index_with_all_blank_lines, which fails without the fix, you get following output:
~
PARA 1596606895
PARA 713924259
PARA 251759511
PARA 1764544288
MOD from 0
added 3: 713924259 251759511 1764544288
removed 1: 1596606895
MOD from 1
added 2: 251759511 1764544288
removed 0:
vis 1596606895
0: 713924259
1: 251759511
2: 1764544288
~

You see that there are created four paragraphs. The first in the constructor of GenericEditableStyledDocumentBase. The other three are the three empty lines that the test adds to the text area.

Below is the modification before trim. 1596606895, which was created in the constructor, should be removed. Three new paragraphs should be added.
~
MOD from 0
added 3: 713924259 251759511 1764544288
removed 1: 1596606895
~

Below is the modification after "buggy" trim. So the first paragraphs from the "added" and from the "removed" lists was removed. IOW the paragraph 1596606895 stays in the list of paragraphs and is not replaced by 713924259, which causes the bugs in #777 and #788.
~
MOD from 1
added 2: 251759511 1764544288
removed 0:
~

And here is the output of visibleParToAllParIndex(). You see the visible paragraph is still at 1596606895, but 1596606895 is no longer in the list of paragraphs.
~
vis 1596606895
0: 713924259
1: 251759511
2: 1764544288
~

And here is the output with fixed trim. The initial paragraph 1950984118 is removed and the visible paragraph is correctly at 1714800072.
~
PARA 1950984118
PARA 1714800072
PARA 2258300
PARA 1082323298
MOD from 0
added 3: 1714800072 2258300 1082323298
removed 1: 1950984118
MOD from 0
added 3: 1714800072 2258300 1082323298
removed 1: 1950984118
vis 1714800072
0: 1714800072
1: 2258300
2: 1082323298
~

In the above case, the new trim does nothing and you might have the impression that we could remove it at all. This is not true. E.g. run JavaKeywordsDemo, and enter some text. You'll notice modifications that change from e.g. 20 paragraphs to 0 paragraphs, which is caused by codeArea.setStyleSpans(...):
~
MOD from 0
added 20: 847350689 785255895 7744232 2068574306 900904319 1479772899 293092209 790259324 503986205 1051951086 1858103546 311810776 1569772103 2021235220 353614425 695001401 1924467897 1177059777 1593099859 485837059
removed 20: 847350689 785255895 7744232 2068574306 900904319 1479772899 293092209 790259324 503986205 1051951086 1858103546 311810776 1569772103 2021235220 353614425 695001401 1924467897 1177059777 1593099859 485837059
MOD from 20
added 0:
removed 0:
~