Project

General

Profile

Bug #7646

FILL-IN: NPE during the PASTE of non-text clipboard contents

Added by Vladimir Tsichevski almost 3 years ago. Updated 5 months ago.

Status:
Merge Pending
Priority:
Normal
Assignee:
Vladimir Tsichevski
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Hynek Cihlar
production:
No
env_name:
topics:

Related issues

Related to User Interface - Bug #7515: FILL-IN: editing dates issues WIP

History

#1 Updated by Vladimir Tsichevski almost 3 years ago

  • Status changed from New to WIP
  • Assignee set to Vladimir Tsichevski

Scenario:

  1. there is no text contents in system-wide clipboard (for example, the user copied an image to clipboard)
  2. the user selects a FILL-IN and calls paste (by typing Ctrl-V, for example).
  3. the client crashes with NPE.

The problem root is in the FillInGuiImpl, l.1012:

         String txt = gd.clipboardContents();

         for (char ch : txt.toCharArray())
         {
            ...

In case there is no text clipboard contents, the txt variable is null.

#2 Updated by Greg Shah almost 3 years ago

Do we need a task for this? The fix seems trivial.

#3 Updated by Vladimir Tsichevski almost 3 years ago

Greg Shah wrote:

Do we need a task for this? The fix seems trivial.

I think we don't. I can fix this in the trunk after some testing.

#4 Updated by Greg Shah almost 3 years ago

Instead of applying the fix to trunk directly, I prefer to just include the fix in another branch where you are already working.

#5 Updated by Vladimir Tsichevski almost 3 years ago

Greg Shah wrote:

Instead of applying the fix to trunk directly, I prefer to just include the fix in another branch where you are already working.

OK

#6 Updated by Vladimir Tsichevski almost 3 years ago

  • Related to Bug #7515: FILL-IN: editing dates issues added

#8 Updated by Vladimir Tsichevski almost 3 years ago

Greg Shah wrote:

Instead of applying the fix to trunk directly, I prefer to just include the fix in another branch where you are already working.

Will be fixed in the 7019a branch.

#9 Updated by Vladimir Tsichevski almost 2 years ago

The NPE in a FILL-IN does not happen anymore, but the same problem still exists for EDITOR widgets.

#10 Updated by Vladimir Tsichevski almost 2 years ago

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

Vladimir Tsichevski wrote:

The NPE in a FILL-IN does not happen anymore, but the same problem still exists for EDITOR widgets.

Fixed in 7646a rev. 15384. Please, review.

#11 Updated by Greg Shah over 1 year ago

  • reviewer Hynek Cihlar added

#12 Updated by Hynek Cihlar over 1 year ago

  • Status changed from Review to Internal Test

Code review 7646a. The change is good and safe.

Vladimir, are you ready to merge this?

#13 Updated by Vladimir Tsichevski over 1 year ago

Hynek Cihlar wrote:

Code review 7646a. The change is good and safe.

Vladimir, are you ready to merge this?

Yes

#14 Updated by Vladimir Tsichevski over 1 year ago

The branch 8646a rebased to the latest trunk.

#15 Updated by Vladimir Tsichevski 7 months ago

Vladimir Tsichevski wrote:

Hynek Cihlar wrote:

Code review 7646a. The change is good and safe.

Vladimir, are you ready to merge this?

Yes

This change has been pending merge for over a year. Ping!

#16 Updated by Hynek Cihlar 6 months ago

Vladimir Tsichevski wrote:

This change has been pending merge for over a year. Ping!

I added myself to the watchers so that I don't miss future updates.

Vladimir, please rebase, if no issues, we can merge to trunk.

#17 Updated by Vladimir Tsichevski 6 months ago

Hynek Cihlar wrote:

Vladimir Tsichevski wrote:

This change has been pending merge for over a year. Ping!

I added myself to the watchers so that I don't miss future updates.

Vladimir, please rebase, if no issues, we can merge to trunk.

The branch 7646a is based on trunk revision 15640 (over a year old).
Since then the trunk repository has been broken, so rebasing is not possible.

I'll create a new branch 7646b instead.

#18 Updated by Vladimir Tsichevski 6 months ago

Vladimir Tsichevski wrote:

I'll create a new branch 7646b instead.

Done

#19 Updated by Vladimir Tsichevski 6 months ago

This issue was partially addressed later in #10085 (similar null check added for no-current-selection case).
However, an NPE is still raised when pasting non-text content into an editor with active selection.

#20 Updated by Vladimir Tsichevski 6 months ago

Updated to r16411: Removed now unnecessary null check in handleInsertAtCursor(). Added minor style improvements.

#21 Updated by Hynek Cihlar 5 months ago

I'm running ChUI regression tests on 7646b. Vladimir, is there any other regression testing needed?

#22 Updated by Vladimir Tsichevski 5 months ago

Hynek Cihlar wrote:

I'm running ChUI regression tests on 7646b. Vladimir, is there any other regression testing needed?

There is no point in testing for ChUI regressions in this branch: only the GUI part was affected by the change.

#23 Updated by Hynek Cihlar 5 months ago

Vladimir Tsichevski wrote:

Hynek Cihlar wrote:

I'm running ChUI regression tests on 7646b. Vladimir, is there any other regression testing needed?

There is no point in testing for ChUI regressions in this branch: only the GUI part was affected by the change.

Good, I'm aborting the ChUI tests. Let me know, when you finish GUI regression tests.

#24 Updated by Vladimir Tsichevski 5 months ago

Hynek Cihlar wrote:

Vladimir Tsichevski wrote:

Hynek Cihlar wrote:

I'm running ChUI regression tests on 7646b. Vladimir, is there any other regression testing needed?

There is no point in testing for ChUI regressions in this branch: only the GUI part was affected by the change.

Good, I'm aborting the ChUI tests. Let me know, when you finish GUI regression tests.

I've tested this with GUI. The original issue is no longer observed in either FILL-IN or EDITOR widgets.

#25 Updated by Vladimir Tsichevski 5 months ago

7646b rebased to the latest trunk and is r16424.

#26 Updated by Vladimir Tsichevski 5 months ago

The change in 7646b committed and pushed as r16425.

#27 Updated by Hynek Cihlar 5 months ago

  • Status changed from Internal Test to Merge Pending

Vladimir Tsichevski wrote:

The change in 7646b committed and pushed as r16425.

Please merge to trunk.

#28 Updated by Vladimir Tsichevski 5 months ago

Hynek Cihlar wrote:

Vladimir Tsichevski wrote:

The change in 7646b committed and pushed as r16425.

Please merge to trunk.

Done as r16426.

Also available in: Atom PDF