Project

General

Profile

Bug #10085

Fix offset cursor when pressing (CTRL+V) in an editor

Added by Ionut-Mihai Simioniuc about 1 year ago. Updated 8 months ago.

Status:
Test
Priority:
Normal
Assignee:
Razvan-Nicolae Chichirau
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Andrei Iacob, Hynek Cihlar, Sergey Ivanovskiy, Vladimir Tsichevski
production:
No
env_name:
topics:

long-strings-serialize-patch.diff Magnifier (2.83 KB) Vladimir Tsichevski, 07/15/2025 11:31 AM

Related issues

Related to User Interface - Bug #10344: StringIndexOutOfBoundsException in editor when deleting text after complex operations New

History

#2 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • Status changed from New to WIP
  • Assignee set to Ionut-Mihai Simioniuc
  • reviewer Andrei Iacob, Vladimir Tsichevski added

The following test case:

def var fi as character view-as editor size 20 by 2.
def button btn.
def frame fmain fi.
on ctrl-v anywhere do:
    message "Test".
    fi:edit-paste() in frame fmain.
end.
enable all with frame fmain.
wait-for close of current-window.

demonstrates that after pressing CTRL+V into the editor, the cursor is in the wrong position, at the text.length - 1 position.

Created task branch 10085a

#3 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • % Done changed from 0 to 80
  • Status changed from WIP to Review

Committed in branch 10085a, with revision 15961.

Please review!

#4 Updated by Andrei Iacob about 1 year ago

Considering that getCursorOffset returns 1-based index and setCursorOffset also expects 1-based index, it makes sense.

Look into how undo.recordPastedText and undo.processUndo work with the new index.

#5 Updated by Ionut-Mihai Simioniuc about 1 year ago

I have tested the changes with 

 undo.recordPastedText(offset, txt);
and 
 undo.recordPastedText(offset - 1, txt);
and both have the same behavior.
I did not find 
 undo.processUndo
in EditorGuiImpl.java.

#6 Updated by Andrei Iacob about 1 year ago

Ionut-Mihai Simioniuc wrote:

I did not find [...] in EditorGuiImpl.java.

EditorGUiImpl$UndoableEditor.processUndo.

#7 Updated by Ionut-Mihai Simioniuc about 1 year ago

I tested it,

EditorGUiImpl$UndoableEditor.processUndo.

and it looks good.

#8 Updated by Ionut-Mihai Simioniuc about 1 year ago

I found a bug: when deleting 2 or more times, the 

deletedText
variable becomes empty, and CTRL+Z does nothing,even though it should. I trying to investigate to fix this.

#9 Updated by Greg Shah about 1 year ago

  • reviewer Hynek Cihlar, Sergey Ivanovskiy added

#10 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • Status changed from Review to WIP
  • reviewer deleted (Hynek Cihlar, Sergey Ivanovskiy)

#11 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • reviewer Hynek Cihlar, Sergey Ivanovskiy added

#12 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • % Done changed from 80 to 60

I fixed the issue.

I found a bug: when deleting 2 or more times, the [...] variable becomes empty, and CTRL+Z does nothing,even though it should. I trying to investigate to fix this.

Now I am trying to fix the cyclic {CTRL+V , CTRL+Z, ...} problem.
And also to fix the problem with backspace.

#13 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • Status changed from WIP to Review
  • % Done changed from 60 to 100

I have resolved the issue with "Backspace|CTRL+DELETE|DELETE". The problem was that when the offset was at position 0 or at the end of text, there were no characters available to be deleted,

deleteText
was reset, which should not have happened. Due to this incorrect reset, bugs appeared in the undo function. When pressing CTRL+Z, the deleted text was appended to the current text.

Another issue that I have fixed was the inability to perform cyclic undo operations. Before the fix, when undo was executed multiple times in succession, the variables were reset incorrectly and emptied each time.

Committed 10085a revision 15962.

Please review!

#14 Updated by Vladimir Tsichevski about 1 year ago

Code review for 10085a revision 15962.

I’ve found an issue in the undo functionality where pasting text doesn’t reset the deletedText field, leading to unexpected behavior:

1. Insert some text into an editor, e.g., a.
2. Select this text and press Ctrl-X. The text is cut as expected, and the editor now contains no text.
3. Press Ctrl-V. The text is inserted back as expected, and the editor now shows a.
4. Repeat this select/cut/paste sequence one more time. Everything works as expected, and the editor still shows a.
5. Now press Ctrl-Z. In OE, this leaves a visible and selected. In FWD, this incorrectly inserts another copy of the text, resulting in aa.

Additionally, I noticed a minor code style violation in EditorGuiImpl.java. A history entry line exceeds the limit with 112 characters (the maximum is 110).

#15 Updated by Ionut-Mihai Simioniuc about 1 year ago

Committed 10085a revision 15963.
Fixed the issue wih CTRL+X when deleteText not cleaned up as it should have been

Please review, Vladimir Tsichevski!

#16 Updated by Ionut-Mihai Simioniuc about 1 year ago

Please review, Hynek Cihlar!

#17 Updated by Vladimir Tsichevski about 1 year ago

Ionut-Mihai Simioniuc wrote:

Committed 10085a revision 15963.
Fixed the issue wih CTRL+X when deleteText not cleaned up as it should have been

Please review, Vladimir Tsichevski!

Done. The #10085-14 issue is gone now.

Another scenario:

1. Insert some text into an editor, e.g., a.
2. Select this text and press Ctrl-X. The text is cut as expected, and the editor now contains no text.
3. Press Ctrl-V two times. The text is inserted twice as expected, and the editor now shows aa.
4. Now press Ctrl-Z. In OE, this reverts to the previous value a visible and selected. In FWD, no change happens, and the text stays as aa.

#18 Updated by Ionut-Mihai Simioniuc about 1 year ago

Committed 10085a revision 15964.
I fixed that bug. I verify if it is the first "CTRL+V" pressing then we overwrite insertedText variable. Also ensure that prevCaretOffset is updated, and lastResetOffset is updated only the first time of paste. If it is pressed in a row another "CTRL+V", then we append that text and update prevCaretOffset.

      private void recordPastedText(int key, int cursorOffset, String text)
      {
         // Reset inserted text for first paste
         if (lastKey == Keyboard.KA_PASTE)
         {
            insertedText += text;
         }
         else
         {
            insertedText = text;
            lastResetOffset = cursorOffset;
         }
         prevCaretOffset = cursorOffset;
         lastKey = key;
         showDelete = true;
         wasDelete = false;
      }

I also fixed bug at the start of writing in editor, without deleting or doing another action. If you are typing and after that press "CTRL+Z" for two times, the previous insertedText value was inserted again. This is now fixed with the following change:

     // Ensure to not decrease less than 1.
     if (offset >= 1)
     {
        offset--;
     }

I also fixed the alternating bug between deletedText and InsertedText. If I was typing, after press "CTRL+Z" and typing again, instead of viewing the previous data, was inserted : "". It's fixed by overloading this method with "lastText" param :

      private void reset(int key, boolean printable, String lastText)
      {
         prevCaretOffset = editor.getCursorOffset();
         lastResetOffset = editor.getCursorOffset();
         lastKey = key;
         lastPrintable = printable;
         deletedText = lastText;
         insertedText = "";
         showDelete = true;
         wasDelete = false;
      }

Please review, Vladimir Tsichevski!

#19 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • Status changed from Review to WIP
  • % Done changed from 100 to 90

I found another scenario.
Scenario 2:
1. Insert some text into an editor, e.g., a.
2. Select the text and copy with "CTRL + C".
3. Delete a.
4. Insert some text into an editor, e.g., bbb.
5. Paste with "CTRL + V" into bb-b.
6. Press "CTRL + Z".
7. Insert/type a.
8. Press "CTRL + Z".
The bug has appeared. In 4gl, it looks like 'bbab', but in FWD it appears as 'bbaab'.

I've already fixed it in commit 15965.

#20 Updated by Ionut-Mihai Simioniuc about 1 year ago

Scenario 3:
1. Insert some text into an editor, e.g., a.
2. Select the text and copy with "CTRL + C".
3. Delete a.
4. Insert some text into an editor, e.g., bbb.
5. Paste with "CTRL + V".
6. Press "CTRL + Z".
The bug has appeared. In 4gl, it looks like 'a', but in FWD nothing happens. It remains 'bbba'.

#21 Updated by Ionut-Mihai Simioniuc about 1 year ago

I've fixed this issue. Everything looks ok when pressing "CTRL + V" and "CTRL + Z".

Committed 10085a revision 15966.

Please review, Vladimir Tsichevski!

#22 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • Status changed from WIP to Review
  • % Done changed from 90 to 100

#23 Updated by Ionut-Mihai Simioniuc about 1 year ago

Hi Vladimir, I know this task isn't a top priority, but if you get a chance, could you please take a look at it? Thanks a lot!

#24 Updated by Ionut-Mihai Simioniuc about 1 year ago

I did rebase to the trunk. The commit revision has changed to the 16000

#25 Updated by Vladimir Tsichevski about 1 year ago

UTFDataFormatException in EDITOR with Long Text

When an EDITOR contains text longer than 65535 characters, the client crashes with a java.io.UTFDataFormatException.

The exception originates from the ObjectOutputStream code, which enforces a 65535-byte limit strings:

  void writeUTF(String s, long utflen) throws IOException {
    if (utflen > 0xFFFFL) {
        throw new UTFDataFormatException();
    }
    writeShort((int) utflen);
    if (utflen == (long) s.length()) {
        writeBytes(s);
    } else {
        writeUTFBody(s);
    }
}

This issue may have broader implications beyond this specific task.

#26 Updated by Vladimir Tsichevski about 1 year ago

Ctrl-Z Undo Fails in EDITOR After Cut and Paste

1. In an EDITOR, insert text (e.g., a).
2. Select the text and press Ctrl-X to cut it. The editor becomes empty, as expected.
3. Insert new text (e.g., 123).
4. Select the new text and press Ctrl-V to paste. The text is replaced with a, as expected.
5. Press Ctrl-Z to undo the paste.

In OpenEdge (OE), Ctrl-Z reverts the editor to the previous state, showing 123 with the text selected.
In FWD, Ctrl-Z has no effect, and the editor remains with a.

#27 Updated by Vladimir Tsichevski about 1 year ago

Ionut-Mihai Simioniuc wrote:

I did rebase to the trunk. The commit revision has changed to the 16000

You probably forget to push the rebased branch back to trunk. The version I see is still 15966.

#28 Updated by Ionut-Mihai Simioniuc about 1 year ago

Ctrl-Z Undo Fails in EDITOR After Cut and Paste

I have fixed this issue.

You probably forget to push the rebased branch back to trunk. The version I see is still 15966.

I have pushed the changes, now you should see them.

Committed 10085a revision 16001.

#29 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • % Done changed from 100 to 90
  • Status changed from Review to WIP

Scenario 4:
1. Insert some text into an editor, e.g., a.
2. "CTRL + X" on selected text.
3. Insert some text into an editor, e.g. 123.
4. "CTRL+V" in -> 12_3.
5. "CTRL+Z"
6. "CTRL+V" at the end of 123. e.g. 123a.
7. "CTRL+Z" -> The bug appears.
In 4gl, it looks like '123', but in FWD is '12'.

Scenario 5:
1. Insert some text into an editor, e.g., a.
2. "CTRL + X" on selected text.
3. Insert some text into an editor, e.g. 123.
4. "CTRL+V" in -> 1_23.
5. "CTRL+Z"
6. "CTRL+V" in -> 12_3.
7. "CTRL+Z" -> The bug appears.
In 4gl, it looks like '123', but in FWD is '13'.

#30 Updated by Ionut-Mihai Simioniuc about 1 year ago

  • Status changed from WIP to Review
  • % Done changed from 90 to 100

I've fixed scenarios 4 and 5.

Committed 10085a revision 16002.

Please review, Vladimir Tsichevski!

#31 Updated by Ionut-Mihai Simioniuc about 1 year ago

Vladimir Tsichevski wrote:

UTFDataFormatException in EDITOR with Long Text

When an EDITOR contains text longer than 65535 characters, the client crashes with a java.io.UTFDataFormatException.

The exception originates from the ObjectOutputStream code, which enforces a 65535-byte limit strings:
[...]

This issue may have broader implications beyond this specific task.

The problem comes from this method:

public static void writeString(ObjectOutput out, String value)
   throws IOException
   {
      out.writeByte(value == null ? 0 : 1);
      if (value != null)
      {
 ---->   boolean asObject = value.length() > 0xFFFF; // maximum length is 65535 for writeUTF
         out.writeByte(asObject ? 1 : 0);
         if (asObject)
         {
            out.writeObject(value);
         }
         else
         {
            out.writeUTF(value);
         }
      }
   }

Instead of checking the length of bytes, we check the length of strings, which is incorrect.
For example, if we have a string with 30.000 characters, and each character requires 3 bytes to be stored in UTF_8, the total size becomes 30.000 * 3 -> 90.000 bytes.
In this scenario, the writeUTF method is invoked instead of writeObject, because we use .length() to count characters. However, writeUTF has a limitation: it can only handle up to 65535 bytes.
Since the actual byte size exceeds this limit, an error occurs.

Solution is:

public static void writeString(ObjectOutput out, String value)
   throws IOException
   {
      out.writeByte(value == null ? 0 : 1);
      if (value != null)
      {
         byte[] valueBytesLength = value.getBytes(StandardCharsets.UTF_8);
         boolean asObject = valueBytesLength.length > 0xFFFF; // maximum length is 65535 for writeUTF
         out.writeByte(asObject ? 1 : 0);
         if (asObject)
         {
            out.writeObject(value);
         }
         else
         {
            out.writeUTF(value);
         }
      }
   }

Now the problem is solved.
I have tested with : "€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€" x 30769
and now everything looks great.

Committed 10085a revision 16003.

Please review, Vladimir Tsichevski!

#32 Updated by Andrei Iacob about 1 year ago

Ionut/Vladimir, regarding UTFDataFormatException, also consider #10193, task reported by a customer.
In that case, exception was thrown when a text with length bigger than 65535 was selected.

#33 Updated by Vladimir Tsichevski about 1 year ago

Ionut-Mihai Simioniuc wrote:

Vladimir Tsichevski wrote:

UTFDataFormatException in EDITOR with Long Text

When an EDITOR contains text longer than 65535 characters, the client crashes with a java.io.UTFDataFormatException.

The exception originates from the ObjectOutputStream code, which enforces a 65535-byte limit strings:
[...]

This issue may have broader implications beyond this specific task.

The problem comes from this method:
[...]

Instead of checking the length of bytes, we check the length of strings, which is incorrect.
For example, if we have a string with 30.000 characters, and each character requires 3 bytes to be stored in UTF_8, the total size becomes 30.000 * 3 -> 90.000 bytes.
In this scenario, the writeUTF method is invoked instead of writeObject, because we use .length() to count characters. However, writeUTF has a limitation: it can only handle up to 65535 bytes.
Since the actual byte size exceeds this limit, an error occurs.

Solution is:
[...]

Now the problem is solved.
I have tested with : "€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€€" x 30769
and now everything looks great.

Committed 10085a revision 16003.

Please review, Vladimir Tsichevski!

The issue arises because we continue to use ObjectOutputStream.writeUTF() to serialize strings of any length, a practice not supported by this function.
In my opinion, we should consider the following options:

  • Restrict the maximum supported string length to 65535 characters. This approach would be cumbersome to enforce consistently and is generally an suboptimal solution.
  • Modify the FWD serialization code to utilize the ObjectOutputStream class across all APIs (de facto already in use, thus legitimizing this), replacing ObjectOutput.writeUTF() with ObjectOutputStream.writeString(), which supports longer strings.

#34 Updated by Ionut-Mihai Simioniuc about 1 year ago

The method writeString is private, as you can see in the code:

private void writeString(String str, boolean unshared) throws IOException {
    handles.assign(unshared ? null : str);
    long utflen = bout.getUTFLength(str);
    if (utflen <= 0xFFFF) {
        bout.writeByte(TC_STRING);
        bout.writeUTF(str, utflen);
    } else {
        bout.writeByte(TC_LONGSTRING);
        bout.writeLongUTF(str, utflen);
    }
}

Since writeString is invoked internally, we could use ObjectOutput.writeObject() instead, as that method indirectly handles long strings by calling writeString.

However, I’m not sure what you meant by "de facto already in use, thus legitimizing this" . Are you suggesting to replace all calls to .writeString with ObjectOutput.writeObject ?

Greg, could you help me please to clarify this? Thank you!

#35 Updated by Greg Shah about 1 year ago

Greg, could you help me please to clarify this? Thank you!

I think you are asking Vladimir to answer this?

#36 Updated by Ionut-Mihai Simioniuc about 1 year ago

Hi Vladimir, may I open another task for the formatting issue? It’s not related to the CTRL+V fill-in. And because of the formatting issue, I can't make progress on the original problem.

#37 Updated by Vladimir Tsichevski about 1 year ago

Ionut-Mihai Simioniuc wrote:

Hi Vladimir, may I open another task for the formatting issue? It’s not related to the CTRL+V fill-in. And because of the formatting issue, I can't make progress on the original problem.

Do you mean the issue with java.io.UTFDataFormatException? Please, do open another issue for this problem.

#38 Updated by Vladimir Tsichevski about 1 year ago

Please, rebase the branch to the latest trunk.

#39 Updated by Vladimir Tsichevski about 1 year ago

Ionut-Mihai Simioniuc wrote:

The method writeString is private, as you can see in the code:
[...]

Since writeString is invoked internally, we could use ObjectOutput.writeObject() instead, as that method indirectly handles long strings by calling writeString.

Exactly so.

However, I’m not sure what you meant by "de facto already in use, thus legitimizing this" . Are you suggesting to replace all calls to .writeString with ObjectOutput.writeObject

My mistake. We do not need this really. Please, ignore my proposal about ObjectOutputStream.

I've attached the patch long-strings-serialize-patch.diff, which removes the StringSerializer entirely and reverts string serialization to rely on the ObjectSerializer.

As previously agreed, create a new issue to address this problem.

#40 Updated by Ionut-Mihai Simioniuc about 1 year ago

So, Vladimir, regarding the original task, would it be okay to include it in the internal test?

#41 Updated by Ionut-Mihai Simioniuc 12 months ago

So could I move on the Internal Test?

#42 Updated by Vladimir Tsichevski 12 months ago

Ionut-Mihai Simioniuc wrote:

So could I move on the Internal Test?

I think yes.

#43 Updated by Ionut-Mihai Simioniuc 12 months ago

  • Status changed from Review to Internal Test

#44 Updated by Razvan-Nicolae Chichirau 12 months ago

A regression was found on another customer application, but it can be replicated in a basic editor widget. The problem arises in EditorGuiImpl$UndoableEditor.recordDeletedText(), where the added getChars() call will throw an StringIndexOutOfBoundsException.

Stack trace:

The bug seems to replicate only when mixing up CTRL+V, CTRL+Z and a lot of newlines. After that, pressing backspace will throw the error. Unfortunately, I cannot reproduce the issue constantly, but I advised Ionut to continue the investigation on this end.

It's caused by .resolveContent() returning the full editor text which, for example, is 50 chars long, while .contentLength() returns a value much lower than that (<50). Normally, we are trying to get the editor text from offset 49 to 50, but the end is modified like so in EditorGuiImpl.getChars():

endOffset   = Math.min(endOffset - 1, editor.contentLength());

, so we'll try to get a substring from 49 to 20 for example.

Important note: ATM I don't know if it's a bug caused or unveiled by 10085a. Either way, it should be treated immediately as it crashes the entire client.

#45 Updated by Ionut-Mihai Simioniuc 12 months ago

I tested with trunk, rev 16059 and I was able to reproduce the same issue. So the issue is not from 10085a.

#46 Updated by Ionut-Mihai Simioniuc 12 months ago

  • Related to Bug #10344: StringIndexOutOfBoundsException in editor when deleting text after complex operations added

#47 Updated by Ionut-Mihai Simioniuc 12 months ago

I tested with two other client applications and everything seems fine.

#48 Updated by Ionut-Mihai Simioniuc 12 months ago

I think it's ready for merge.

#49 Updated by Alexandru Lungu 8 months ago

  • Assignee changed from Ionut-Mihai Simioniuc to Razvan-Nicolae Chichirau

Razvan, please rebase 10085a and redo testing. Let me know when this will be ready to be merged.

#51 Updated by Razvan-Nicolae Chichirau 8 months ago

Testing that passed successfully:
  • ChUI regression tests
  • Hotel GUI + 2 customer apps smoke-test
  • Unit-tests from customers
  • Checked all editor-related tests from testcases project
  • Verified isolated tests such as #10085-2

If this is enough, let's merge 10085a.

#52 Updated by Razvan-Nicolae Chichirau 8 months ago

+ Alex

#53 Updated by Alexandru Lungu 8 months ago

  • Status changed from Internal Test to Merge Pending

I agree. Lets merge 10085a to trunk now.

#54 Updated by Razvan-Nicolae Chichirau 8 months ago

  • Status changed from Merge Pending to Test

Branch 10085a was merged into trunk as rev. 16300 and archived.

Also available in: Atom PDF