Qt 5.13 seems to break sorting the book list

Bug #1834989 reported by Eli Schwartz on 2019-07-02
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
calibre
Undecided
Unassigned

Bug Description

As per https://bugs.archlinux.org/task/63063

Building calibre from a git clone using a system Qt 5.13 results in a
book list where no columns can be sorted, and attempting to click on the
columns simply does not register. Immediately upon opening, the book
list has a different sort from the accustomed one -- it is being sorted
from oldest to newest on the "date" column.

As an aside, I'm also seeing new debug messages:
QCssParser::parseColorValue: Specified color without alpha value but
alpha given: 'rgb 255, 255, 255, 0%'

 affects calibre

--
Eli Schwartz

I can reproduce on OpenSUSE Tumbleweed.

This looks like a bug in PyQt, as best as I can tell,
QTableView.sortbyColumn() is not calling the sort() method on the
model, probably is calling the base class implementation instead.

Should be reported upstream, sadly I am totally swamped at the moment.

 status invalid

Changed in calibre:
status: New → Invalid
Kovid Goyal (kovid) wrote :

Actually, no it is a combination of calling sortByColumn and blocking
signals on the table header, which indicates the regression is in Qt
rather than PyQt. Since getting stuff fixed in Qt is a sisyphean task,
I'll just work around it in calibre.

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

 status fixreleased

Changed in calibre:
status: Invalid → Fix Released

Thanks!

BTW I reported that the issue was with a system Qt 5.13 because although
Arch Linux updated to PyQt5 5.12.3 at the same time, I was able to
duplicate the issue with PyQt5 5.12.2 and Qt 5.13 as well -- and the
PyQt5 version has worked with calibre just fine in the past.

Fun fact: PyQt5 was already horribly broken right out of the gate, see
https://www.riverbankcomputing.com/pipermail/pyqt/2019-July/041896.html

calibre had the misfortune of being a python2-pyqt5 program rather than
a python3-pyqt5 program, so it hit this issue which made the simple act
of `python2 -c 'import PyQt5.QtCore'` do an immediate coredump.

Kovid Goyal (kovid) wrote :

Yeah, I saw that bug as well. It will be nice when the move to python 3 is done, and we can stop worrying about stuff like this. :)

I am currently focused on stripping out Qt WebKit from calibre, another huge pain.

Eli Schwartz (eschwartz) wrote :

I reported the bug to Qt as https://bugreports.qt.io/browse/QTBUG-76850, so I guess we'll see if it gets fixed soon.

Eli Schwartz (eschwartz) wrote :

Cross-posting a reply from the Qt ticket:

"The insulting commit is d0f909f8dbdd8594b0d950822f0e7ab8728da513 but I don't think it's a problem within Qt but due to the fact that the signals from the horizontal header are blocked when sortByColumn() is called.

This internal implementation behavior was changed and is now in sync with QTreeView::sortByColumn() which always relied on the sortIndicatorChanged() signal from the header.

So don't block the signals from the header and it will work as expected."

Maybe it's best that calibre does not use this function anymore.

Eli Schwartz (eschwartz) wrote :

So I guess this was a behavior change as a result of this commit -- which marked the function as deprecated: https://github.com/qt/qtbase/commit/d0f909f8dbdd8594b0d950822f0e7ab8728da513

Kovid Goyal (kovid) wrote :

Yeah, as I said in #3 I already changed calibre to not use sortByColumn in places where header signal are possibly blocked. It seems rather weird that Qt would decide to break compatibility like this after over a decade, but, shrug.

Eli Schwartz (eschwartz) wrote :

Indeed. Just thought I'd mention here that it seems this is "a feature, not a bug". No clue *why*, but that seems to be the "what".

"Richard Moe Gustavsen *closed* an issue as *Invalid*

Yes, I agree! Poking into QTableView to stop signals from being emitted
will naturally cause it to break (even if it happen to work at an
earlier point). If this was supposed to be supported, I think we would
have to redesign many widgets."

We have our answer, I guess. :)

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers