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

[python-api] - do not use core context menus for WindowXML containers #9605

Merged
merged 2 commits into from Apr 14, 2016

Conversation

Projects
None yet
3 participants
@phil65
Copy link
Member

commented Apr 12, 2016

@tamland
This fixes the issues I mentioned in #9595
Core context menu doesnt really work with listitems added to WindowXML instances, so let script authors deal with that via onAction().

@tamland

This comment has been minimized.

Copy link
Member

commented Apr 12, 2016

im guessing this has something to do with the CGUIMediaWindow 'Interceptor'. So every time media window is changed, it breaks..

@phil65

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2016

yup, correct, anything which updates the container of the mediawindow breaks it.
I guess to fix all issues, I also have to intercept MSG_GUI_UPDATE ? That one also breaks the current listing. (happens when addons get updated, or by manually triggering the builltin "Container.Refresh").
I know that this is all quite hacky, but I dont see any better way to get that stuff into a usable shape (I guess I am the only one who uses these parts at all :) )

@tamland

This comment has been minimized.

Copy link
Member

commented Apr 12, 2016

Yep. I'll trust you with this. Not touching that class with a ten foot pole:)

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Apr 12, 2016

/me hands over an 11 foot pole ;)

@phil65

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2016

I now completely intercept GUI_MSG_NOTIFY_ALL, which fixes the remaining issues I had.
@tamland , does that look fine enough from 10 foot away? :)

// GUI_MSG_NOTIFY_ALL breaks container content, so intercept it.
return true;
}
break;

This comment has been minimized.

Copy link
@tamland

tamland Apr 14, 2016

Member

no need for break and the brackets

@@ -370,6 +377,9 @@ namespace XBMCAddon
PulseActionEvent();
return true;
}
// the core context menu can lead to all sort of issues right now when used with WindowXMLs, so lets intercept the corresponding message
else if (controlClicked->IsContainer() && message.GetParam1() == ACTION_CONTEXT_MENU)
return true;

This comment has been minimized.

Copy link
@tamland

tamland Apr 14, 2016

Member

double indent

@tamland

This comment has been minimized.

Copy link
Member

commented Apr 14, 2016

Other than cosmetics, it looks ok ..from a distance

@phil65 phil65 force-pushed the phil65:windowxml_contextmenu branch from 709a269 to 89b59f3 Apr 14, 2016

@phil65

This comment has been minimized.

Copy link
Member Author

commented Apr 14, 2016

took care of cosmetics, thx for review.
jenkins build this please.

@phil65 phil65 merged commit b169d4a into xbmc:master Apr 14, 2016

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
default Merged build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.