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

Fix #328003 score property crash #10211

Closed
wants to merge 5 commits into from

Conversation

jonnymoo
Copy link
Contributor

@jonnymoo jonnymoo commented Jan 4, 2022

Resolves: https://musescore.org/en/node/328003

Following accessibility changes saving score properties causes MuseScore to crash

  • [ x] I signed CLA
  • [ x] I made sure the code in the PR follows the coding rules
  • [ x] I made sure the code compiles on my machine
  • [ x] I made sure there are no unnecessary changes in the code
  • [ x] I made sure the title of the PR reflects the core meaning of the issue you are solving
  • [ x] I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • [ x] I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • [ x] I created the test (mtest, vtest, script test) to verify the changes I made

…rrect but I cant instance QWidgets in a test yet
…lication so widgets can be instanced in gtests. Overall fix for crashing is to allow QLabels and QLineEdits to be used in properties following Eisms accessibility fixes musescore@1aa603c
@jonnymoo jonnymoo changed the title 328003 score property crash Fix #328003 score property crash Jan 8, 2022
@jonnymoo
Copy link
Contributor Author

jonnymoo commented Jan 10, 2022

The reason why the build failed was I incorrectly put qualifiers on my class declaration - see https://stackoverflow.com/questions/5642367/extra-qualification-error-in-c

I've fixed with the latest commit.

Apparently visual studio is more than happy with this, but other compilers are bit more strict. I've removed them. I haven't put the properties item class back into the properties page and simplified because I struggled to mock out the whole of the page - so it is easier to test the smaller class. If you think this is not good practise, I of course won't take offense but might need a bit of help in how to create a moc screen and inject a project with properties etc.

@jonnymoo jonnymoo closed this Jan 13, 2022
@jonnymoo jonnymoo deleted the 328003-ScorePropertyCrash branch January 13, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants