Current row values getting updated in the above row with latest Chrome Version 128.0.6613.120 (Official Build) (64-bit)

[pavanmajji03]

Describe the bug

Chrome Version 128.0.6613.120 (Official Build) (64-bit)
OS: Windows 10
Angular version: 15
angular-slickgrid: 5.6.4

Reproduction

1. Edit a cell value in row
2. Click Enter

Actual: The value is getting updated in the above cell in above row

Expectation

The value should get updated in the same cell in same row

Environment Info

Chrome Version 128.0.6613.120 (Official Build) (64-bit)
OS: Windows 10
Angular version: 15
angular-slickgrid: 5.6.4

Validations

• [X] Follow our Code of Conduct
• [X] Read the HOWTO - Quick Start.
• [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• [X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
• [X] The provided reproduction is a minimal reproducible example of the bug.

[ghiscoding]

Unless you provide a full repro, I will close this issue. There are plenty of unit tests and E2E tests covering editing, I would be surprised of any issues. You should make sure that the Ids that you are editing are the expected ones.

I also don't provide any support for older versions, only the latest major version is supported

[pavanmajji03]

Yes I agree, you wouldn't support but I want to fix this in my application anyhow. Can we connect if possible on any channel or somewhere if you suggest, I have debugged a detail and found some things. It would be really great if we could connect, but give me some time to have stackblitz or some sort of reproduction.

And as well is there a problem for the marking the current row if the method getBoundingClientRect() dosent give the correct y value. Because I observed in previous version of the chrome and current version of the chrome I am getting a difference of ~15px for y position Will this be a problem in finding the current active row? ref:chrome issue which might relate to above

[ghiscoding]

This is an Open Source project, I am not being paid for this and so I am not available via any channels. You need to provide a full repro or I just won't help (sorry but I already spent way too much time on this free project).

You can simply open the Stackblitz link on the main readme page, then open any of the example and reproduce the error you're having and that is the way to provide a full repro.

to get the active row, it seems that the function getActiveCell() will give you that

[ghiscoding]

Also running Cypress E2E tests with latest version which also has latest Chrome works fine.

Testing manually with latest Chrome version also works fine on my side

So unless you provide better repro I will really close this issue and who knows, it might have been an issue that was fixed a while ago in newer versions of my libs, it's hard to know and that is why I don't provide support for older version (mostly because it's an open source and free project)

[pavanmajji03]

@ghiscoding Issue reproduced even on the latest version, attached the recording.

https://github.com/user-attachments/assets/27a4c48b-0e98-4fa2-8d28-2ffe47cc19eb

Please check and do let me know if there any solution for this problem

[ghiscoding]

You're purposely changing the body zoom level and it was never mentioned in your issue at least not until you demo it in the video, so your issue wasn't filled in correctly. However, the question is why are you doing this? Is it really the only approach that you have?

Because the problem is that SlickGrid relies on the row height (that can also be changed via the grid options) to do lots of internal calculations, for example it uses it to find the row of the cell you clicked and when you change the zoom level then you are offsetting all of these calculations. For example, the cell edit that you're having difficulties with, would have to change the code with something like this

https://github.com/ghiscoding/slickgrid-universal/blob/7a6dd0659cb425af59a68c1f7a08241f987a5505/packages/common/src/core/slickGrid.ts#L3260C3-L3262C4

protected getRowFromPosition(y: number): number {
-    return Math.floor((y + this.offset) / this._options.rowHeight!);
+   const zoom = parseInt(document.body.style.zoom) || 100;
+   const currentRowHeight = this._options.rowHeight! / 100 * zoom;
+   return Math.floor((y + this.offset) / currentRowHeight);
}

but that is just 1 fix (also probably doesn't cover all zooming ways either), and like I said SlickGrid relies on these calculations to do a lot of other stuff (find row by x,y coordinates, row selection, row dragging, etc...), so not just the editing as you can see.

If however you use the browser zoom then it works fine.

I don't really know what to say here, I think that you're looking for trouble by playing with the zoom and if we want the lib to support it then there would 50 lines of code to look into and I can't be 100% sure to cover them all. I'm not sure that I want to proceed with such change and even if I do, I will only fix latest version so even if I fix it, you won't get the fix until you upgrade yourself to the latest version (unless you npm patch the lib). According to this SO How to detect page zoom level in all modern browsers?, there's a lot of different ways to get this wrong as well

@zewa666 do you have any suggestions on this matter?

I think my position would be... don't use zoom

[pavanmajji03]

@ghiscoding Thank you for the reply

Yes, in the application the setup was such a way on the zoom, so I figured this cause after I raised the issue.

But just wanted to call out one important thing, This same scenario works perfectly fine in older version of chrome/ edge browsers without a problem as internally this package uses getBoundingClientRect() which is providing a different result when compared to older version when the y, top values were not changing drastically with the zoom style setting.

Which brought me raise an issue in chromium project as well Ref to the issue: https://issues.chromium.org/issues/365913984

And also I have created a stackblitz to showcase the same scenario. Ref: https://stackblitz.com/edit/vitejs-vite-mqasgn?file=main.js

Hope this adds on or helps to take a right decision for your changes

CC: @zewa666

[zewa666]

I agree that supporting different zoom levels would be out of scope here as the core library itself doesnt respect it. but also neither most likely do other dependencies you bet on in combo with slickgrid related to positioning.

my take would be to add an init check if the zoom level is changed and console.warn the user about subpar support for that.

[ghiscoding]

I took another look and tried to do some code changes to support it but quickly realized that there's a lot of indirect issues and so I agree with @zewa666 and I will not support a zoom level that is not 100% and simply show a console warning via this Slickgrid-Universal PR 1676

Browser zoom is working fine though, so that is what users should use