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

[gui] fix bug when opening playlists with smartplaylisteditor several times #9545

Merged
merged 1 commit into from Oct 6, 2016

Conversation

phil65
Copy link
Contributor

@phil65 phil65 commented Apr 3, 2016

Example: call

ActivateWindow(smartplaylisteditor,special://skin/playlists/inprogress_movies.xsp)

several times and you will notice that the rule list gets longer and longer because the previous rules were not cleared.
@Montellese for guidance please, not sure what the best place is to do the .Reset()

@Montellese
Copy link
Member

Can you check what happens when you open the smartplaylist editor for a specific xsp (With above call) and then open it without one with ActivateWindow(smartplaylisteditor)? From the looks of your change you will still see the rules from the initial xsp which we'd have to handle as well.

@phil65
Copy link
Contributor Author

phil65 commented Apr 7, 2016

ah ok, I thaught this codepath would only be used when the additional playlistpath param was passed via ActivateWindow().
Should I just move the call up some few lines, right before

if (!startupList.empty())

?

@Montellese
Copy link
Member

Probably yeah.

@phil65
Copy link
Contributor Author

phil65 commented Oct 5, 2016

@Montellese tried this again but moving that line up introduces other issues (when called from EditPlaylist(), m_playlist is set up before GUI_MSG_WINDOW_INIT already and thus gets overwritten there again)

I now tried to do the Reset() in GUI_MSG_WINDOW_DEINIT and everything worked fine during a quick test. Is this fix okay-ish?

@phil65 phil65 added Type: Fix non-breaking change which fixes an issue v17 Krypton labels Oct 5, 2016
@Montellese
Copy link
Member

Actually this is something I've wandered a lot when working with dialogs. A lot of dialogs have to be manually reset in the code before they can be used. I always wondered why they weren't automatically reset when they were closed.

It should be fine as long as m_playlist isn't used by whoever opened the dialog in the first place (which shouldn't be the case because it's not public anyway).

@phil65
Copy link
Contributor Author

phil65 commented Oct 6, 2016

Actually this is something I've wandered a lot when working with dialogs. A lot of dialogs have to be manually reset in the code before they can be used. I always wondered why they weren't automatically reset when they were closed.

Agreed.

It should be fine as long as m_playlist isn't used by whoever opened the dialog in the first place (which shouldn't be the case because it's not public anyway).

Ok. jenkins build this please.

@phil65 phil65 merged commit b57c736 into xbmc:master Oct 6, 2016
@phil65 phil65 deleted the smartplaylisteditor_fix branch October 6, 2016 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants