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

Skins: append / when setting skin base path #4151

Merged
merged 1 commit into from Jul 27, 2021

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Jul 27, 2021

This ensures stylesheet icon urls without leading / are transformed to correct paths.

Regression from #3877, noticed with DarkMetal https://mixxx.discourse.group/t/crates-playlists-history-etc-vanish-from-dark-metal/22704/ : the user assumed the library was broken because the sidebar expand arrow icons were missing...

So my hasty conclusion was wrong

// Note: It's safe to use the base path after parseSkin() has updated it
// (parseLaunchImage() appends "/" earlier)
return style.replace("url(skin:", "url(" + m_pContext->getSkinBasePath());

because the skin base path is also set by LegacySkinParser::parseSkin() without trailing /

Appending the / will result in // but this is filtered by Qt later on (assuming since leading / are used for the LateNight launch image and that works on all platforms)

Proposed Changelog entry

- Fix support for relative paths in the skin system which caused missing images in third-party skins. #4151

Tthis ensures stylesheet icon urls without leading / are transformed correctly
@ronso0 ronso0 added the skins label Jul 27, 2021
@ronso0 ronso0 added this to the 2.3.1 milestone Jul 27, 2021
@ronso0 ronso0 requested a review from Holzhaus July 27, 2021 11:10
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

If it works, then let's use this simple fix.

Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@Holzhaus Holzhaus merged commit 6b09f1f into mixxxdj:2.3 Jul 27, 2021
@ronso0 ronso0 deleted the skinstyles-fix-url-transform branch July 27, 2021 13:35
@Holzhaus Holzhaus added the changelog This PR should be included in the changelog label Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog minor bug skins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants