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

[builtins] fix bad access if GetWindow() is called with virtual window id #6990

Merged
merged 1 commit into from Apr 20, 2015

Conversation

xhaggi
Copy link
Member

@xhaggi xhaggi commented Apr 20, 2015

As pointed out by @mkortstiege this will fix a null access if we call g_windowManager.GetWindow(iWindowID) with a virtual window id e.g. WINDOW_MUSIC -> 10005.

@xhaggi
Copy link
Member Author

xhaggi commented Apr 20, 2015

jenkins build this please

@xhaggi xhaggi added the Type: Fix non-breaking change which fixes an issue label Apr 20, 2015
@xhaggi xhaggi added this to the Isengard 15.0-beta1 milestone Apr 20, 2015
@@ -300,7 +300,8 @@ void CBuiltins::GetHelp(std::string &help)
bool CBuiltins::ActivateWindow(int iWindowID, const std::vector<std::string>& params /* = {} */, bool swappingWindows /* = false */)
{
// don't activate a window if there are active modal dialogs
if (g_windowManager.HasModalDialog() && !g_windowManager.GetWindow(iWindowID)->IsDialog())
if (g_windowManager.HasModalDialog() &&
!(g_windowManager.GetWindow(iWindowID) != NULL && g_windowManager.GetWindow(iWindowID)->IsDialog()))

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@xhaggi
Copy link
Member Author

xhaggi commented Apr 20, 2015

@FernetMenta refactored the active modals validation into CGUIWindowManager::ActivateWindow_Internal()

@FernetMenta
Copy link
Contributor

looks good imo

@xhaggi
Copy link
Member Author

xhaggi commented Apr 20, 2015

squashed .. jenkins build this please

@xhaggi
Copy link
Member Author

xhaggi commented Apr 20, 2015

any objections?

@mkortstiege
Copy link
Member

+1

xhaggi added a commit that referenced this pull request Apr 20, 2015
[builtins] fix bad access if GetWindow() is called with virtual window id
@xhaggi xhaggi merged commit e1da274 into xbmc:master Apr 20, 2015
@xhaggi
Copy link
Member Author

xhaggi commented May 5, 2015

@FernetMenta there are different issues where windows can't be activated because of open modals. Take a look at http://trac.kodi.tv/ticket/15960. The first approach with PR #6828 was to only block user-interactive activation (via built-ins) of new windows while a modal dialog is open. After this we block also internal activations like switch to fullscreen which results in the issue described in #7022.

IMO we should revert to the old behavior and only block user-interactive activation (via built-ins). I would create a new method for that in CGUIWindowManager, place the check for active modals there and call this method in CBuiltins. Or do you have any other idea to fix it.

@xhaggi xhaggi deleted the builtin-fix-nullpointer branch May 5, 2015 10:10
@FernetMenta
Copy link
Contributor

@xhaggi the issue #7022 is somewhere else, it has a problem by design.

EDIT: not 7022 but the Stereoscopic manager

@FernetMenta
Copy link
Contributor

@xhaggi go ahead with your suggestion. I don't have a better idea

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants