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

Export Playlist file naming issue #11327

Closed
webdb22 opened this issue Mar 4, 2023 · 12 comments · Fixed by #11332
Closed

Export Playlist file naming issue #11327

webdb22 opened this issue Mar 4, 2023 · 12 comments · Fixed by #11332

Comments

@webdb22
Copy link

webdb22 commented Mar 4, 2023

Bug Description

When exporting a playlist using "Export Playlist", the dialogue comes up with the playlist name, e.g "MyPlaylist" and the "m3u" extension, e.g. "MyPlalist.m3u" as a default, which is useful and fine.

If you switch the file format using the combo box (M3U, M3U8, PLS,...), the exported file is in the correct (selected) file format, but the file name has still the "m3u" included, e.g. "MyPlaylist.m3u.pls".

The expected behaviour would be, that if the file format is switched using the combo box, the extension "m3u" should be replaced by the selected file format extension, so the file name should end up being "MyPlaylist.pls" (without the "m3u")

Version

2.3.4

OS

Ubuntu 22.04

@webdb22 webdb22 added the bug label Mar 4, 2023
@ronso0
Copy link
Member

ronso0 commented Mar 5, 2023

This was last touched by #4531, and IIRC it was only on Linux where this simple file dialog was causing trouble...

The extension that is finally selected is simply appended to the file name, compared to replacing every .az09 string, see
#4531 (comment)

So we need to check if the file name ends with .[one of the fileFilters], then append the extension.

@ronso0
Copy link
Member

ronso0 commented Mar 6, 2023

@webdb22 Please test #11332 to confirm the fix.

@webdb22
Copy link
Author

webdb22 commented Mar 6, 2023

@ronso0 Thanks a lot for providing a fix on such short notice!

Please excuse my ignorance, unfortunately I'm by no means a developer, how to the test the fix you're referring to? Any package I could download and install you can point me to? Thanks!

@ronso0
Copy link
Member

ronso0 commented Mar 6, 2023

Oh, sorry, here are the instructions how test PRs with pre-built binaries provided by Github https://github.com/mixxxdj/mixxx/wiki/Testing#github-pull-requests, and #11332 is the PR with the fix.
Also read the chapter above about backups.

@daschuer
Copy link
Member

daschuer commented Mar 7, 2023

I cannot confirm the issue with Ubuntu Focal and Qt 5.12.8
But there is an issue that when I try to export a crate, there is no extension added at all.
If this is an issue of different Qt version, we need also test with our windows and macOS version.

@webdb22
Copy link
Author

webdb22 commented Mar 7, 2023

@ronso0 Thanks for pointing me to the instructions!

I've followed the instructions up to the "Artifacts", unfortunately for Ubuntu there is only "18.04" and "20.04" listed, it seems "22.04" is missing.

Nevertheless I've tried to install the version for "20.04" on my Ubuntu 22.04:
$ sudo apt install ./mixxx-2.3.4-2-g5f8d28670b.deb

but it fails with:

The following packages have unmet dependencies:
 mixxx : Depends: libprotobuf-lite17 but it is not installable
         Depends: libssl1.1 (>= 1.1.0) but it is not installable

and mixxx doesn't get installed. Looks like it is requiring "older" libraries which 22.04 doesn't provide.

Any chance you could provide a .deb for Ubuntu 22.04 so I can test it?

@ronso0
Copy link
Member

ronso0 commented Mar 7, 2023

@daschuer This is not about the extension displayed in the dialog, it's about the extension of the resulting playlist file after selecting a different extension than the default m3u.

@webdb22
Copy link
Author

webdb22 commented Mar 7, 2023

But there is an issue that when I try to export a crate, there is no extension added at all.

@daschuer I can confirm the behavior in the UI for exporting a playlist file from a Crate that no file extension is added in the UI, but the playlist file created has the correct file extension as selected in the UI. This seems to me a different issue, but nevertheless related to the initial issue.

From a usability point of view both "Export Playlist" and "Export Crate" should behave similar, i.e. either display the extension in the UI and export the file with the selected extension only; or don't display the extension in the UI and just add the correct extension to the file when exported (like "Export Crate" is currently doing).

@ronso0
Copy link
Member

ronso0 commented Mar 7, 2023

I've followed the instructions up to the "Artifacts", unfortunately for Ubuntu there is only "18.04" and "20.04" listed, it seems "22.04" is missing.

@webdb22 Yes, this is unfortunate. It's been reported (@daschuer?) that you can to install these packages manually (no replace, just add), at least libprotobuf-lite17. You can try that, but don't bend over since for this specific bug it doesn't matter too much because it can be reproduced by others.

@ronso0
Copy link
Member

ronso0 commented Mar 7, 2023

As I alreayd stated in #11332 since the file dialog seems to be broken (on Linux with both Qt 5.12.8 and 5.15.*), it feels like we're now pretty close to reimplementing QFileDialog::getSaveFileName and would "just" need to hook up to the filterSelected signal to get good UX inside the dialog (= immediately replace/append the selected extension) ...

I'm not to keen on doing that, but if it finally gets us a clean UX 🤷‍♂️

@webdb22
Copy link
Author

webdb22 commented Mar 9, 2023

@ronso0 I've created a new instance with Ubuntu 20.04 (Focal Fossa) and installed Mixxx from PR #11332

Good news, the behaviour is now as expected, when switching the file type, the file extension is changed accordingly, no "m3u" left in the file name, well done!

Thanks a lot for providing this fix on such short notice!

ExportPlaylist_UbuntuFocalFossa

@daschuer
Copy link
Member

Fixed in #11332

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants