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

Fix sometimes select plugin setting from context menu not works problem. #2255

Merged
merged 1 commit into from
Feb 21, 2013

Conversation

ulion
Copy link
Contributor

@ulion ulion commented Feb 18, 2013

currently, when we are in a plugin source dir, if selected item's path is not a plugin/script path, e.g. raw final http url, select 'Plugin Settings' from the item's context menu items will now show the plugin's setting dialog.
that's because the current code only use the selected item's path's hostname to get plugin/addon, it will fail in condition I mentioned.
I checked codes to show the context menu, there's condition like this:

    if (item->IsPlugin() || item->IsScript() || m_vecItems->IsPlugin())
      buttons.Add(CONTEXT_BUTTON_PLUGIN_SETTINGS, 1045);

obviously we will lost if the item is an http url item in a plugin dir in current code, so here's the fix PR.

@jmarshallnz
Copy link
Contributor

Looks fine, though agreed with t-nelson that it could be made clearer.

@ulion
Copy link
Contributor Author

ulion commented Feb 18, 2013

added temp variable and comment as @t-nelson suggested.

@ulion
Copy link
Contributor Author

ulion commented Feb 21, 2013

if no objection, I will merge this after 2 days.

jmarshallnz added a commit that referenced this pull request Feb 21, 2013
Fix sometimes select plugin setting from context menu not works problem.
@jmarshallnz jmarshallnz merged commit dfcc04d into xbmc:master Feb 21, 2013
@ulion ulion deleted the fix_plugin_setting_not_show_problem branch April 6, 2013 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants