Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MU3] Fix deprecation warnings with Qt 5.15.2 and some palette qml files #7388

Closed
wants to merge 1 commit into from

Conversation

Jojo-Schmitz
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz commented Feb 2, 2021

These warnings (lots of them) show at run time, not at compile time and may be quite irritating esp. for/on Linux, where MuseScore may use a newer Qt version.

Many times during startup and again on palette use:

Warning: qrc:/qml/palettes/Palette.qml:766:13: QML Connections: Implicitly defined onFoo properties in Connections are deprecated. Use this syntax instead: function onFoo(<arguments>) { ... } (qrc:/qml/palettes/Palette.qml:766, )

Once per startup:

Warning: qrc:/qml/palettes/PaletteTree.qml:772:5: QML Connections: Implicitly defined onFoo properties in Connections are deprecated. Use this syntax instead: function onFoo(<arguments>) { ... } (qrc:/qml/palettes/PaletteTree.qml:772, )
Warning: qrc:/qml/palettes/PalettesWidgetHeader.qml:225:5: QML Connections: Implicitly defined onFoo properties in Connections are deprecated. Use this syntax instead: function onFoo(<arguments>) { ... } (qrc:/qml/palettes/PalettesWidgetHeader.qml:225, )

See also https://doc.qt.io/qt-5/qml-qtqml-connections.html

For now a test, need to check the artifacts, which would use the older Qt 5.9.9 (which I don't have installed any more)
As per https://stackoverflow.com/questions/62297192/qml-connections-implicitly-defined-onfoo-properties-in-connections-are-deprecat as of QT 5.15.1 that new warning can get avoided by QT_LOGGING_RULES="qt.qml.connections=false" and that new/changed syntax is not supported by Qt 5.12 :-(

The artifact (for Windows) seems to work just fine though!?
What operations would need to get tested to verify? Selecting a score element and applying a palette element via double click works, as does dragging a palette element to a score element

@Jojo-Schmitz Jojo-Schmitz changed the title [MU3] Fix deprecation wanings with Qt 5.15.2 and some qml files [MU3] Fix deprecation warnings with Qt 5.15.2 and some qml files Feb 2, 2021
@dmitrio95
Copy link
Contributor

dmitrio95 commented Feb 3, 2021

As far as I can tell lack of these operations will not break palettes entirely, so most basic operations will probably indeed work as expected. Certain UX-related features may however break if these handlers would work incorrectly:

  • onHasFocusChanged in PaletteTree.qml ensures that "More" popup gets closed if you click outside of the current palette (or maybe outside of the entire palettes list, but if you select an element in the same palette this popup should stay).
  • Another onHasFocusChanged seems to do a similar thing for the "Add Palettes" list at the panel header.
  • onElementDraggedToScoreView ensures that when some element gets dragged to a score the corresponding cell should not be somehow hidden or moved — this is only relevant if you trigger palette cells reordering by the same drag first.

So these aspects should probably be tested to ensure that they work properly. But as far as I understand these functions will not be considered signal handlers by Qt 5.9 if they are declared as functions so the corresponding features should likely break.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Feb 3, 2021

Thanks
OK, testing with the artifact (for Windows) shows that the first test succeeds: the "More" popup does vanish when clicking outside the palette and stays as long as elements within get selected.
The second tests succeeds too.
As does the third, as far as I can tell.
And all what with Qt 5.9.9, so your assertion that this would break things seems wrong. Seems to indicate that this change is safe, doesn't it?

@vpereverzev
Copy link
Member

I'd prefer to not merge this into 3.x, because it's pretty dangerous and the benefit is small. I also strongly discourage to build the 3.x branch with QT 5.15

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Feb 3, 2021

I tend to agree. Linux maintainers may not though.
I 'd leave it open though, for easy cherry-picking

@RomanPudashkin
Copy link
Contributor

RomanPudashkin commented Feb 5, 2021

I 'd leave it open though, for easy cherry-picking

These warnings are already fixed in master

@Jojo-Schmitz
Copy link
Contributor Author

Yes, but not for 3.x, not on (Linux) builds from distributors that otherwise use Qt 5.15+
Cherry picking tor those distribution maintainers, not for you ;-)

@Jojo-Schmitz Jojo-Schmitz changed the title [MU3] Fix deprecation warnings with Qt 5.15.2 and some qml files [MU3] Fix deprecation warnings with Qt 5.15.2 and some palette qml files Feb 5, 2021
@vpereverzev vpereverzev added the archived PRs that have gone stale but could potentially be revived in the future label Mar 10, 2021
@vpereverzev
Copy link
Member

I'll archive this PR, the link will still persist

@Jojo-Schmitz
Copy link
Contributor Author

Well, if a 3.6.3 is not planned, merging this won't do any harm either...

@Jojo-Schmitz
Copy link
Contributor Author

This PR is something @mirabilos might be interested in, for the Debian ports?

@@ -767,7 +767,7 @@ GridView {
// force not hiding palette cell if it is being dragged to a score
enabled: paletteCell.paletteDrag
target: mscore
onElementDraggedToScoreView: paletteCell.paletteDrag = false
function onElementDraggedToScoreView() { paletteCell.paletteDrag = false; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really?

I’d have thought this…

onElementDraggedToScoreView: function () { paletteCell.paletteDrag = false; }

But if it works… sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does for me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t doubt that ☻ it’s just that JavaScript continues to irritate me, the more I learn about it ☺

@mirabilos
Copy link
Contributor

@vpereverzev we build with whatever Qt version we have in the distros. That is currently:

  • 5.3 on jessie(Debian) ⇒ MuseScore 2.3.2
  • 5.5 on precise(PPA), trusty(PPA), xenial(Ubuntu) ⇒ MuseScore 2.3.2
  • 5.7 on stretch(Debian) ⇒ MuseScore 2.3.2; MuseScore 3.2.3 with disabled plugin functionality
  • 5.9 on bionic(Ubuntu) ⇒ MuseScore 2.3.2 and 3.x
  • 5.11 on buster(Debian) ⇒ MuseScore 2.3.2 and 3.2.3; 3.x via backports later
  • 5.12 on focal(Ubuntu) ⇒ MuseScore 2.3.2 and 3.x
  • 5.14 on groovy(Ubuntu nōn-LTS, dropped soon) ⇒ MuseScore 2.3.2 and 3.x
  • 5.15 on hirsute(Ubuntu nōn-LTS), bullseye(Debian), sid(Debian) ⇒ MuseScore 2.3.2 and 3.x

They all work, for what it’s worth, with the occasional patch; I’m ignoring the deprecation warnings for now since there won’t be a Qt 6 in Debian/Ubuntu in the foreseeable future.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Mar 16, 2021

There shouldn't be any deprecation warnings in the latest 3.x codebase, those had been taken care of quite a while ago (but probably after 3.2.3, see #6813), not in the C++ code and at compile time at least. Just those QML ones from this PR, and at runtime.

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 18, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 24, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 10, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 26, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request May 11, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jun 28, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 27, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 28, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Jul 28, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 13, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 13, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 19, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 19, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 23, 2021
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 5, 2023
@Jojo-Schmitz Jojo-Schmitz deleted the qml-warnings branch May 11, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived PRs that have gone stale but could potentially be revived in the future
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants