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

controllers: Fix crash if controller mapping has no module #2946

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jul 16, 2020

This fixes two bugs that were caused by #2868:

  1. Mixxx didn't check if the JS module path was really a file. If
    you pass in a directory, QtJSEngine::importModule makes Mixxx
    crash (at least on Qt 5.15.0)
  2. If a mapping does not have JS module associated with it, then the
    moduleFileInfo always pointed to the controller directory and
    hence triggered bug 1.

This fixes both of these issues and adds some debug assertions to make
sure it stays like that.

This fixes two bugs that were caused by mixxxdj#2868:

    1. Mixxx didn't check if the JS module path was really a file. If
       you pass in a directory, QtJSEngine::importModule makes Mixxx
       crash (at least on Qt 5.15.0)
    2. If a mapping does not have JS module associated with it, then the
       moduleFileInfo always pointed to the controller directory and
       hence triggered bug 1.

This fixes both of these issues and adds some debug assertions to make
sure it stays like that.
@Holzhaus Holzhaus added this to the 2.4.0 milestone Jul 16, 2020
@Holzhaus Holzhaus requested a review from Be-ing July 16, 2020 11:22
@Holzhaus Holzhaus added this to In progress in 2.4 release via automation Jul 16, 2020
@Be-ing Be-ing merged commit 4669ad2 into mixxxdj:master Jul 16, 2020
2.4 release automation moved this from In progress to Done Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
2.4 release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants