Opening the Edit Metadata dialog causes the book list to scroll down too much

[Jellby]

When opening the Edit Metadata dialog for a single book, it used to be that the book list scrolls to show the selected book in the middle of the list. Since 6.17 what I experience is that the book list scrolls down too much (most of the time to the very bottom) and the selected book is out of sight.

More info in https://www.mobileread.com/forums/showthread.php?t=353762

This happens just by opening the dialog (with keyboard or mouse), no need to make any changes.

My desktop is KDE on Kubuntu 22.04.

I've downgraded to 6.16 and it works fine, upgrade to 6.17 and the wrong scrolling happens.

[Kovid Goyal (kovid)]

Doesn't replicate for me in KDE (current on Arch) with both X11 and
Wayland. Opening the edit metadata single dialog with a single book selected results in
the book being scrolled to the center of the screen as expected.

It may be some interaction between the version of the window manager on
your system and Qt. Hopefully an update will cure it given that I cannot
replicate with up-to-date KDE.

In any case without being able to replicate there is not much I can do
about it. One thing you can try is create a new empty library with no
custom columns add a few books and see if it replicates there. If not
then attach the metadata.db file from your library folder and I will try
to replicate with that.

[Jellby (jellby)]

Unfortunately, it also happens with a new (default) library. Would it be possible for you to test it with a virtual (or real) machine on Kubuntu 22.04? Or would it be possible for me to try to pinpoint which code change is responsible for the behaviour in my system?

[Kovid Goyal (kovid)]

Too much work for me, sorry. I already know which code changes it was
problem is I dont see anything wrong with them to correct. It would be
something from between
55b6205d84 and 7b5024b97a

[Jellby (jellby)]

Please, ignore (for now). I've tried a clean, "portable" version of calibre 6.17 and it works as intended, so it must be something in my calibre config that does it.

[Kovid Goyal (kovid)]

If you can figure out what is causing the bad interaction do let me
know.

[Jellby (jellby)]

The bug appears with eff5ec46d0. I'll try to find the reason.

[Jellby (jellby)]

It's the 'vertical_scrolling_per_row' tweak. If I set it to False, it works fine, but True makes it wrong. Normal scrolling is correct either way.

[Kovid Goyal (kovid)]

That would imply one of the functions that return the height of the
veiport or the row is returning incorrect values on your system

[Jellby (jellby)]

Does it mean you still cannot reproduce it? Anyway, I don't think that's the issue. In a test case where I select the bottom element in the screen (with the list scrolled to the top), this is what I get:

vertical_scrolling_per_row = False

The new position (i.e. what's passed to vsb.setValue()) is 188. The selected row is scrolled to the middle of the screen, so the calculations are correct.

vertical_scrolling_per_row = True

All values are the same, so the new position is also 188. But now passing this to vsb.setValue() seems to have a different meaning, and indeed the list is scrolled down until row #189 is at the top.

[Kovid Goyal (kovid)]

No no our messages crossed. I will look at the tweak later when I have a
moment.

[Jellby (jellby)]

May I suggest the following patch? (Maybe the 'top' and 'bottom' should be modified too, though):

diff --git a/src/calibre/gui2/library/views.py b/src/calibre/gui2/library/views.py
index 144e3108df..15c6ad7f40 100644
--- a/src/calibre/gui2/library/views.py
+++ b/src/calibre/gui2/library/views.py
@@ -1305,8 +1305,10 @@ def scroll_to_row(self, row):
                 vsb.setValue(vertical_position)
             elif pos == 'bottom':
                 vsb.setValue(vertical_position - viewport_height + cell_height)
- else:
+ elif self.verticalScrollMode() == QAbstractItemView.ScrollMode.ScrollPerPixel:
                 vsb.setValue(vertical_position - ((viewport_height - cell_height) // 2))
+ else:
+ vsb.setValue(row - (viewport_height // (cell_height * 2)))

     @property
     def current_book(self):

[Kovid Goyal (kovid)]

Fixed in branch master. The fix will be in the next release. calibre is usually released every alternate Friday.