Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

An exception pops up when pressing the space key on the checkbox cell #12752

Open
Zheng-Li01 opened this issue Jan 10, 2025 · 9 comments · May be fixed by #12821
Open

An exception pops up when pressing the space key on the checkbox cell #12752

Zheng-Li01 opened this issue Jan 10, 2025 · 9 comments · May be fixed by #12821
Assignees
@ricardobossan
Labels
area-controls-DataGridView 🚧 work in progress Work that is current in progress
Milestone
.NET 10.0

Comments

@Zheng-Li01
Copy link
Member

.NET version

.NET SDK 10.0.100-alpha.1.25058.38

Did it work in .NET Framework?

No

Did it work in any of the earlier releases of .NET Core or .NET 5+?

This is not a regression issue, can reproduce all earlier releases of .NET versions (.NET 6.0/7.0/8.0/9.0) & .NET Framework

Issue description

An exception pops up when pressing the space keyboard on the checkbox cell
Image

Steps to reproduce

  1. Clone the https://github.com/Zheng-Li01/winforms/tree/RepoApplication branch, and run the ScratchProject
  2. Tab to checkbox cell
  3. Press the space keyboard on the checkbox cell
  4. Observe the results

Debug info:
Image

Call Stack:

************** Exception Text **************
System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Windows.Forms.DataGridViewCheckBoxCell.NotifyMSAAClient(Int32 columnIndex, Int32 rowIndex) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Controls\DataGridView\DataGridViewCheckBoxCell.cs:line 1016
   at System.Windows.Forms.DataGridViewCheckBoxCell.OnKeyUp(KeyEventArgs e, Int32 rowIndex) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Controls\DataGridView\DataGridViewCheckBoxCell.cs:line 884
   at System.Windows.Forms.DataGridViewCell.OnKeyUpInternal(KeyEventArgs e, Int32 rowIndex) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Controls\DataGridView\DataGridViewCell.cs:line 2928
   at System.Windows.Forms.DataGridView.OnKeyUp(KeyEventArgs e) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Controls\DataGridView\DataGridView.Methods.cs:line 16177
   at System.Windows.Forms.Control.ProcessKeyEventArgs(Message& m) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs:line 9160
   at System.Windows.Forms.DataGridView.ProcessKeyEventArgs(Message& m) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Controls\DataGridView\DataGridView.Methods.cs:line 22376
   at System.Windows.Forms.Control.ProcessKeyMessage(Message& m) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs:line 9202
   at System.Windows.Forms.Control.WndProc(Message& m) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\Control.cs:line 12626
   at System.Windows.Forms.NativeWindow.Callback(HWND hWnd, MessageId msg, WPARAM wparam, LPARAM lparam) in Q:\source\winforms\src\System.Windows.Forms\src\System\Windows\Forms\NativeWindow.cs:line 349
@Zheng-Li01 Zheng-Li01 added area-controls-DataGridView untriaged The team needs to look at this issue in the next triage labels Jan 10, 2025
@Tanya-Solyanik
Copy link
Member

@Zheng-Li01 - thank you for creating a scratch project repro!

@Tanya-Solyanik Tanya-Solyanik added this to the .NET 10.0 milestone Jan 13, 2025
@Tanya-Solyanik Tanya-Solyanik removed the untriaged The team needs to look at this issue in the next triage label Jan 13, 2025
@LeafShi1
Copy link
Member

This issue is similar to issue #12692. Before calling NotifyMSAAClient, the DataGridView should be checked to see if it is not null.

@Tanya-Solyanik
Copy link
Member

@LeafShi1 - thank you! Please prepare the fix and search the source code for similar cases.

@ricardobossan
Copy link
Member

I proceeded to test WinForms's repo main branch latest commit (10.0.100-alpha.1.25064.3) and the issue doesn't reproduce.

Image

I believe this was fixed by #12701, because removing the null check introduced in that PR would cause the issue to happen again.

@Zheng-Li01
Copy link
Member Author

I proceeded to test WinForms's repo main branch latest commit (10.0.100-alpha.1.25064.3) and the issue doesn't reproduce.

Image Image

I believe this was fixed by #12701, because removing the null check introduced in that PR would cause the issue to happen again.

@ricardobossan Could please try using the space keyboard on the checkbox cell to reproduce this issue.

@ricardobossan
Copy link
Member

Thank you for pointing that out, @Zheng-Li01 . I missed the detail about reproducing the issue by pressing the space key. I retested following the issue description more carefully, and I can confirm the issue does reproduce as described.

ricardobossan pushed a commit to ricardobossan/winforms that referenced this issue Jan 22, 2025
Fixes dotnet#12752
fc7d058
@ricardobossan ricardobossan linked a pull request Jan 22, 2025 that will close this issue
Draft
@dotnet-policy-service dotnet-policy-service bot added the 🚧 work in progress Work that is current in progress label Jan 22, 2025
ricardobossan pushed a commit to ricardobossan/winforms that referenced this issue Jan 22, 2025
Add a null check for the DataGridView instance before calling the…
95f04dc 
… `NotifyMSAAClient` method.

Fixes dotnet#12752

## Root Cause

- The issue occurs because the `NotifyMSAAClient` method is called
without checking if the `DataGridView` instance is `null`. This leads to
a potential `NullReferenceException`.

## Proposed changes

- Add a `null` check for the `DataGridView` instance before calling the
`NotifyMSAAClient` method.

## Customer Impact

- Prevents runtime crashes when the `DataGridView` is `null`.

## Regression?

- No

## Risk

- Minimal

## Screenshots

### Before

### After

## Test methodology

- Manual testing

## Test environment(s)

- `10.0.100-alpha.1.25064.3`
@LeafShi1
Copy link
Member

If the following code is executed in dataGridView1_CellPainting, NullReferenceException will also be thrown.

dataGridView1.Rows.Clear();
dataGridView1.Rows.Add(1, 0, "ABC");
dataGridView1.Rows.Add(2, 0, "DEF");

Therefore, before executing PaintGrid, we need to check whether CurrentCell is null.

It is difficult to find other similar cases just by looking at the code, so I dragged a DataGridView control and added some events to it, and added the above code to the events, so that the problem will be exposed when the corresponding events are executed.
@Tanya-Solyanik Do you think this test method is feasible?

@Tanya-Solyanik Tanya-Solyanik changed the title An exception pops up when pressing the space keyboard on the checkbox cell An exception pops up when pressing the space key on the checkbox cell Jan 22, 2025
@Tanya-Solyanik
Copy link
Member

@LeafShi1 @ricardobossan - I would add a null-check before on in PaintGrid. Performance cost is low, and someone might clear the cells in the paint method.

ricardobossan pushed a commit to ricardobossan/winforms that referenced this issue Jan 22, 2025
Prevents runtime crashes when the DataGridView or CurrentCell are…
826374d 
… null.

Fixes dotnet#12752

## Root Cause

- The issue occurs because the `NotifyMSAAClient` method is called
without checking if the `DataGridView` instance is `null`. This leads to
a potential `NullReferenceException`.

## Proposed changes

- Add a `null` check for the `DataGridView` instance before calling the
`NotifyMSAAClient` method.
- Adds another `null` check for the `CurrentCell` property before
calling the `PaintGrid` method.

## Customer Impact

- Prevents runtime crashes when the `DataGridView` or `CurrentCell` are
null.

## Regression?

- No

## Risk

- Minimal

## Screenshots

### Before

### After

## Test methodology

- Manual testing

## Test environment(s)

- `10.0.100-alpha.1.25064.3`
@ricardobossan
Copy link
Member

@LeafShi1 @ricardobossan - I would add a null-check before on in PaintGrid. Performance cost is low, and someone might clear the cells in the paint method.

I have updated the PR to reflect this suggestion.

ricardobossan pushed a commit to ricardobossan/winforms that referenced this issue Jan 22, 2025
Prevents runtime crashes when the DataGridView or CurrentCell are…
3d18116 
… null.

Fixes dotnet#12752

## Root Cause

- The issue occurs because the `NotifyMSAAClient` method is called
without checking if the `DataGridView` instance is `null`. This leads to
a potential `NullReferenceException`.

## Proposed changes

- Add a `null` check for the `DataGridView` instance before calling the
`NotifyMSAAClient` method.
- Adds another `null` check for the `CurrentCell` property before
calling the `PaintGrid` method.

## Customer Impact

- Prevents runtime crashes when the `DataGridView` or `CurrentCell` are
null.

## Regression?

- No

## Risk

- Minimal

## Screenshots

### Before

### After

## Test methodology

- Manual testing

## Test environment(s)

- `10.0.100-alpha.1.25064.3`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ricardobossan ricardobossan

Labels
area-controls-DataGridView 🚧 work in progress Work that is current in progress
Projects
None yet
Milestone
.NET 10.0
Development

Successfully merging a pull request may close this issue.

Fixes Exception being thrown when the space key is pressed on a CheckBoxCell ricardobossan/winforms
4 participants
@Tanya-Solyanik @ricardobossan @Zheng-Li01 @LeafShi1