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

[addons] fix stupid circumstance with addon settings on folder with file extension #17449

Merged
merged 1 commit into from
Mar 14, 2020

Conversation

AlwinEsch
Copy link
Member

@AlwinEsch AlwinEsch commented Mar 7, 2020

Description

Found this when adding settings to vfs.rar.

There called this place with "special://profile/addon_data/vfs.rar", this was seen
by "CFileDirectoryFactory::Create" as a file and tried to open with vfs.rar addon.

This logically leads to an error because it is only a folder.

The slash is left now on the folder so that it is recognized as a folder.

Motivation and Context

Got a big question mark when I added the settings and they weren't saved but worked when addon was disabled.

That these slashes have been omitted has been in history since Kodi's "github". Do not see the need to leave them out and the test went well.

How Has This Been Tested?

Screenshots (if appropriate):

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)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

…ile extension

Found this when adding settings to vfs.rar.

There called this place with "special: //profile/addon_data/vfs.rar", this was seen
by "CFileDirectoryFactory::Create" as a file and tried to open with vfs.rar addon.

This logically leads to an error because it is only a folder.

The slash is left now on the folder so that it is recognized as a folder.
@AlwinEsch
Copy link
Member Author

Is this OK?

@phunkyfish
Copy link
Contributor

Would this also impact python addons?

@AlwinEsch
Copy link
Member Author

AlwinEsch commented Mar 14, 2020

Actually not directly, since it only refers to the storage of the data at .kodi/userdata/addon_data/*/settings.xml and is used to create the folders required there.

Use in this line only https://github.com/xbmc/xbmc/blob/5c96aaa455782c2842c123c7e40c665c8aa736c5/xbmc/addons/Addon.cpp#LL164-L171.

But it would also be good if a second could test it, so that there is not something hidden in it.

Copy link
Contributor

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, I tried it locally and it worked.

@AlwinEsch AlwinEsch merged commit 8d8e164 into xbmc:master Mar 14, 2020
@AlwinEsch
Copy link
Member Author

Thanks a lot!

@AlwinEsch AlwinEsch removed the PR Cleanup: Needs Review This PR is good and ready for a review. Someone please review. label Mar 14, 2020
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Mar 16, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
@AlwinEsch AlwinEsch deleted the Matrix-fix-change branch May 20, 2020 13:13
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
[addons] fix stupid circumstance with addon settings on folder with file extension
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants