Add note to coin control dialog QT5 workaround #7256

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:remove-qt5-fix-ccontrol changing 1 files +1 −1

fanquake commented at 4:47 AM on December 26, 2015: member

I can no longer reproduce the original issue (#3846) with QT 5.5
luke-jr commented at 7:34 AM on December 26, 2015: member

Debian is at 5.3.2 - can someone test if that needs this?
jonasschnelli added the label GUI on Dec 27, 2015
jonasschnelli commented at 7:31 AM on January 2, 2016: contributor

I guess the upstream bug is that one: https://bugreports.qt.io/browse/QTBUG-43473. Maybe it's to early to remove it... will test on Debian and OSX with Qt<5.5...
jonasschnelli commented at 9:12 AM on January 2, 2016: contributor

Tested on stable debian (8.x, just downloaded and did a VM net install). I guess the bug was fixed in Qt5.4. and debian is still on 5.3.x. I guess we should keep the workaround another year.
jonasschnelli commented at 9:13 AM on January 2, 2016: contributor

Without the workaround, selecting a parent will result in partial check of the parent and all it's childs (which is a bug).

<img width="1096" alt="bildschirmfoto 2016-01-02 um 10 11 06" src="https://cloud.githubusercontent.com/assets/178464/12073930/60aff118-b139-11e5-9eff-b9279a08aba0.png">
fanquake commented at 9:16 AM on January 2, 2016: member

Yes that is the work around the bug is fixing, if it's still present the workaround can remain. Should we add a note that it's fixed in 5.4, and as such can be removed once all platforms have upgraded to it?
jonasschnelli commented at 9:18 AM on January 2, 2016: contributor

Yes. And I think we should add a link to the upstream bug in the comment.
Add note to CoinControl Dialog workaround 33877ed3b8
fanquake force-pushed on Jan 2, 2016
fanquake renamed this:
~~Remove QT5 workaround from coin control dialog~~
Add note to coin control dialog QT5 workaround
on Jan 2, 2016
fanquake commented at 9:39 AM on January 2, 2016: member

Changes pushed.
jonasschnelli commented at 9:40 AM on January 2, 2016: contributor

ACK 33877ed3b8c7ee7e814ebee894ccdcc8ce6ab122

in src/qt/coincontroldialog.cpp:None in 33877ed3b8

 410 | @@ -411,7 +411,7 @@ void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column)
 411 |      // todo: this is a temporary qt5 fix: when clicking a parent node in tree mode, the parent node
 412 |      //       including all children are partially selected. But the parent node should be fully selected
 413 |      //       as well as the children. Children should never be partially selected in the first place.
 414 | -    //       Please remove this ugly fix, once the bug is solved upstream.
 415 | +    //       Should be fixed in Qt5.4 and above. https://bugreports.qt.io/browse/QTBUG-43473
 416 |  #if QT_VERSION >= 0x050000

MarcoFalke commented at 12:39 PM on January 2, 2016:

What about

#if QT_VERSION >= 0x050000 && QT_VERSION <= 0x050500

jonasschnelli commented at 12:42 PM on January 2, 2016:

Yes. We have discussed that on IRC and IMO it would look after we are overdoing the workaround. Let's just keep it and remove the WA after one or two years.

MarcoFalke commented at 1:35 PM on January 2, 2016:

My reasoning was that gitian uses qt5.5, so we don't need it in the official releases. People that are using qt-5.3 or qt-5.4, still have it enabled.

paveljanik commented at 1:28 PM on January 3, 2016: contributor
I'd like to see the comment about removing updated and stay there (or moved after the link to Qt bug) - something like
```
Please remove this ugly fix, once fixed Qt is used in all relevant platforms.
```
laanwj commented at 8:21 AM on January 4, 2016: member

utACK
laanwj merged this on Jan 4, 2016
laanwj closed this on Jan 4, 2016
laanwj referenced this in commit 08ab9069af on Jan 4, 2016
fanquake deleted the branch on May 12, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#3846 [Qt] Fix coin control qt5 checkbox bug in tree mode