qt: drop workaround for QTBUG-42503 which was fixed in Qt 5.5.0 #189

pull prusnak wants to merge 1 commits into bitcoin-core:master from prusnak:qtbug-42503 changing 1 files +0 −1

prusnak commented at 3:39 PM on January 19, 2021: contributor

Fixes https://github.com/bitcoin-core/gui/issues/101
qt: drop workaround for QTBUG-42503 which was fixed in Qt 5.5.0
Fixes https://github.com/bitcoin-core/gui/issues/101
a2a3f4cd8d
hebasto commented at 3:43 PM on January 19, 2021: member

@prusnak

Thanks! Do you mind providing a relevant screenshot from a retina display?
laanwj commented at 3:51 PM on January 19, 2021: member

Qt 5.5.1 is the minimum supported according to dependencies.md, so Concept ACK
jarolrod commented at 4:20 PM on January 19, 2021: member

Concept ACK

The workaround was actually introduced in 7247d103ffc2772b6dd997b7aa4d7147a8044d73 where we moved from a warning text to a warning icon. The QPushButton was used since at the time of that PR the QTBUG-42503 was already known. Otherwise, going by the comment "to work around missing HiDPI support of QLabel" a QLabel would have been used.

Since QLabel is no longer affected by this bug, I guess fixing the workaround would mean refactoring the icon from a QPushButton to a QLabel. But, since we have additional logic with the modal overlay, maybe it makes more sense to keep it a QPushButton.
prusnak commented at 5:45 PM on January 19, 2021: contributor

Thanks! Do you mind providing a relevant screenshot from a retina display?

Unfortunately, I have issues with linking the bitcoin-qt binary on my system with retina display due to unrelated error, so I am not able to provide one.
jarolrod commented at 6:39 PM on January 19, 2021: member

@prusnak @hebasto Here are screenshots taken on a Retina (HiDPI) display (2020 MacBook Pro).

While the line that is removed in this PR is no longer needed, I fail to see how it "fixes" the workaround. The workaround was to use a disabled QPushButton because QLabel had a regression where it failed to scale by the devicePixelRatio (this is no longer the case, see: patch). The fix would not be to continue to use the QPushButton unless it is decided to keep it.

Master <img width="666" alt="master" src="https://user-images.githubusercontent.com/23396902/105077516-a06d3480-5a5a-11eb-815d-027d7f666655.png">

PR <img width="600" alt="pr" src="https://user-images.githubusercontent.com/23396902/105077542-a82cd900-5a5a-11eb-9da1-39ad8d52a950.png">
jonasschnelli commented at 9:41 AM on January 26, 2021: contributor

Tested ACK a2a3f4cd8d53e33b0e1b1aeff073b096ade034e9 -
<img width="664" alt="Bildschirmfoto 2021-01-26 um 10 40 43" src="https://user-images.githubusercontent.com/178464/105828026-f6922880-5fc2-11eb-8fd6-1305e397bdf4.png">
jonasschnelli merged this on Jan 26, 2021
jonasschnelli closed this on Jan 26, 2021
prusnak deleted the branch on Jan 26, 2021
sidhujag referenced this in commit d72a209295 on Jan 26, 2021
bitcoin-core locked this on Aug 16, 2022

Contributors

Linked (view graph)

#101 Drop workaround for fixed QTBUG-42503