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

Don't hide parent folder icon when browsing by watched movie/episode #9042

Merged
merged 1 commit into from
Feb 10, 2016

Conversation

razzeee
Copy link
Member

@razzeee razzeee commented Feb 3, 2016

When you filter for Watched in the video library. You'll loose the goto parent folder icon, rendering you unable to go up in the hierarchy via the menu.

This fixes the problem.

@razzeee razzeee added Component: Video v17 Krypton Type: Fix non-breaking change which fixes an issue labels Feb 3, 2016
@Montellese
Copy link
Member

Don't we have a similar logic somewhere else which also has to take care of season flattening etc? IIRC I've seen similar code with the same exceptions somewhere else.

@razzeee
Copy link
Member Author

razzeee commented Feb 4, 2016

Having slept over it I think we should/could probably do it the same way we're handling the "* all albums" stuff now?

Will have to search for that other code then. Btw it seems that the flattening code also has a problem, as it seems to still add the "season" level to the "history" for browsing back up. Not sure if I named those concepts correctly as I haven't read up on them.

@Montellese
Copy link
Member

We could inject the parent item using a list item modifier but it would obviously affect all views / nodes and not just season view. What might be an issue is that the list modifier has no idea about the watched / unwatched state.

@razzeee
Copy link
Member Author

razzeee commented Feb 4, 2016

I wouldn't think it will be and I think we want this everywhere. As 99% of our menus have a parent. The important info is basically hasParent yes/no

@razzeee
Copy link
Member Author

razzeee commented Feb 8, 2016

So should I try to change this @Montellese or are we good with this PR?

@Montellese
Copy link
Member

We can leave it like this for now (will also make backporting easier) but would be nice if we could someday refactor the two very similar implementations into one.

@razzeee
Copy link
Member Author

razzeee commented Feb 9, 2016

Not sure how I feel about backporting.

jenkins build this please

@razzeee razzeee added this to the Krypton 17.0-alpha1 milestone Feb 9, 2016
@razzeee
Copy link
Member Author

razzeee commented Feb 9, 2016

I would like to merge this tomorrow, so speak up if anyone sees a problem

razzeee added a commit that referenced this pull request Feb 10, 2016
Don't hide parent folder icon when browsing by watched movie/episode
@razzeee razzeee merged commit bdffd34 into xbmc:master Feb 10, 2016
@razzeee razzeee deleted the bugfix/parent-folder branch February 10, 2016 22:01
@MilhouseVH
Copy link
Contributor

This PR is causing a segfault when navigating into empty folders with "Show parent folder items" disabled/unchecked.

Without this PR there is no problem.

To reproduce:
  1. Add an empty folder to sources.xml, eg:
        <source>
            <name>Local Videos</name>
            <path pathversion="1">/home/neil/videos/</path>
            <allowsharing>true</allowsharing>
        </source>
  1. Contents of folder:
$ ls -la /home/neil/videos/
total 8
drwxrwxr-x  2 neil neil 4096 Feb 27 13:49 .
drwxr-xr-x 45 neil neil 4096 Feb 27 13:49 ..
  1. Disable Show parent folder items in Settings > Appearance > File lists

  2. Navigate into empty folder Videos > Files > Local Videos, and Kodi will segfault.

crashlog

(I intended to create a trac ticket for this, but I can't login to kodi.trac.tv right now - login issues)

@popcornmix
Copy link
Member

Funny, I has a crash that sounded like this last night. Let's check if there's a crashlog...
http://sprunge.us/BXLC

Yep - looks like the same one. I think I clicked on video add-ons which was empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Fix non-breaking change which fixes an issue v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants