Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Random crashes in GenericStyleArea.visibleParToAllParIndex #777

Closed
kszilagyi opened this issue Oct 1, 2018 · 6 comments · Fixed by #795 or #918
Closed

Random crashes in GenericStyleArea.visibleParToAllParIndex #777

kszilagyi opened this issue Oct 1, 2018 · 6 comments · Fixed by #795 or #918
Labels

Comments

@kszilagyi
Copy link

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
Copy link
Author

Apparently resizing the window (sometimes?) fixes it.

@lhttjdr
Copy link

lhttjdr commented Nov 30, 2018

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
Copy link
Collaborator

Jugen commented Nov 30, 2018

@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
Copy link

lhttjdr commented Dec 1, 2018

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

@Jugen
Copy link
Collaborator

Jugen commented Dec 3, 2018

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 !

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

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

JFormDesigner pushed a commit to JFormDesigner/RichTextFX that referenced this issue Dec 21, 2018
…on (issues FXMisc#777 and FXMisc#788)

(borrowed code from ReactFX, which uses same license as RichTextFX)
@JFormDesigner
Copy link
Contributor

@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:

System.out.println("PARA "+System.identityHashCode(this));

To GenericStyledArea.visibleParToAllParIndex():

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():

for (...) {
    p(mod);
    mod = mod.trim();
    p(mod);
...
}

Here is p(mod):

private void p(MaterializedListModification<Paragraph<PS, SEG, S>> 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:

Jugen pushed a commit that referenced this issue Feb 20, 2019
* bug: getVisibleParagraph does not work with blank lines
* trim paragraphs using reference instead of object comparison (issues #777 and #788)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants