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 parent directory ".." not exiting plugins providing multiple types #14409

Merged
merged 2 commits into from
Sep 12, 2018

Conversation

garbear
Copy link
Member

@garbear garbear commented Sep 9, 2018

This fixes unable being able to exit plugins by selecting ".." at the plugin's root when the plugin provides multiple types.

When a plugin provides multiple types, it is launched with the context as URL options, e.g. plugin://addon.id/?content_type=video.

When the ".." at the plugin root is selected, plugins with a single type updated to plugin:// and exit. This is the expected behavior.

Plugins with multiple types, however, update with plugin://addon.id/, which refreshes the root window and doesn't exit.

To fix this, we make the parent directory of the URL with options but no file equal to plugin://, which exits the plugin.

Also includes a small validation check.

Motivation and Context

Reported here: garbear#94

How Has This Been Tested?

Tested on Windows 10 with IAGL. Selecting ".." correctly exits the plugin.

This may have side effects, but I'm not aware of any.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

When a plugin provides multiple types, we pass the context as a URL
option, e.g. plugin://addon.id/?content_type=video.

Simply removing the URL options to obtain the parent path causes us to
refresh the plugin root, whereas single-type plugins will exit the plugin
when ".." is selected.
@garbear garbear added Type: Fix non-breaking change which fixes an issue v18 Leia labels Sep 9, 2018
@garbear
Copy link
Member Author

garbear commented Sep 10, 2018

Any Python guys wanna chime in? Do any plugins use URL options as a subdirectory?

@garbear
Copy link
Member Author

garbear commented Sep 12, 2018

Confirmed fixed in garbear#94

@garbear garbear merged commit ed0618f into xbmc:master Sep 12, 2018
@garbear garbear deleted the fix-plugin-exit branch September 12, 2018 20:24
@AkariDN
Copy link
Contributor

AkariDN commented Sep 13, 2018

@garbear sorry for late reply, but yes, your PR changes plugin parenting rules and breaks some video database functionality. If you only want to make the parent directory of the URL with options but no file equal to plugin://, but your changes also removes both filename and options if file is not empty and leaves options if file is empty.

// wrong, should be plugin://addon.id/filename/
plugin://addon.id/filename/?options -> plugin://addon.id/
// correct
plugin://addon.id/filename/ -> plugin://addon.id/
// wrong, should be plugin://
plugin://addon.id/?options -> plugin://?options
// correct
plugin://addon.id/ -> plugin://

@ksooo
Copy link
Member

ksooo commented Sep 14, 2018

@garbear this also breaks the guiinfo label "ListItem.Path" (and possibly others) for plugin items. See https://forum.kodi.tv/showthread.php?tid=330696&pid=2767502#pid2767502 ff

@garbear
Copy link
Member Author

garbear commented Sep 14, 2018

@ksooo Fixed by #14434, just awaiting a comment

garbear added a commit that referenced this pull request Sep 20, 2018
Fix displaying watched items in plugins and restore functionality after #14409
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 v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants