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

Navigationfix when closing addons #12623

Merged
merged 2 commits into from Aug 5, 2017
Merged

Conversation

a1rwulf
Copy link
Member

@a1rwulf a1rwulf commented Aug 4, 2017

This adds a new message to trigger PREVIOUS_WINDOW from a python addon.
When calling "close" from an addon we now trigger the new message instead of ACTIVATE_WINDOW.

Motivation and Context

Scenario:

  • You started up kodi and went into the tvshow section.
  • You have access to an addon there and open it.
  • The addon is not a dialog window but a normal window.
  • When you hit back to close the addon, you are in the tvshow section again.
  • When you hit back again, you assume to be taken to the home screen again, instead
    you are in the folder view of the MyVideoNav.xml

This happens because the ACTIVATE_WINDOW initializes the window without a start directory.
I can not think about any case where you want to not go "back" when closing an addon.

Maybe anyone can judge if this change is correct!?

How Has This Been Tested?

Tested with kodi master and a windowed addon.

Types of change

  • [ x ] Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@Paxxi Do I need any more documentation when I add new Appmessenger messages?
@xhaggi You've been in the reviewer suggestions ;)

This fixes navigation being messed up when you called a windowed
addon from somewhere else than the home screen.
@xhaggi xhaggi self-requested a review August 4, 2017 08:29
Copy link
Member

@xhaggi xhaggi left a comment

Choose a reason for hiding this comment

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

IMO looks good and makes totally sense because addon windows behaves different and Window::close() should be the same as CGUIWindow::OnBack().

bool CGUIWindow::OnBack(int actionID)
{
  g_windowManager.PreviousWindow();
  return true;
}

@xhaggi xhaggi added Component: Add-ons Type: Fix non-breaking change which fixes an issue Component: GUI engine Type: Improvement non-breaking change which improves existing functionality v18 Leia labels Aug 4, 2017
@a1rwulf
Copy link
Member Author

a1rwulf commented Aug 4, 2017

Cool.
Just in case anyone wonders:
If your addon is a Dialog, close is handled by WindowDialogMixin.cpp which sends some other special message that ends up to be a OnBack later on, see:
https://github.com/xbmc/xbmc/blob/master/xbmc/interfaces/legacy/WindowDialogMixin.cpp#L45

@a1rwulf a1rwulf changed the title RFC: Navigationfix when clsoing addons Navigationfix when clsoing addons Aug 4, 2017
@a1rwulf a1rwulf changed the title Navigationfix when clsoing addons Navigationfix when closing addons Aug 4, 2017
@MartijnKaijser MartijnKaijser merged commit 794694a into xbmc:master Aug 5, 2017
@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Aug 5, 2017
@a1rwulf a1rwulf deleted the navigationfix branch November 22, 2017 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Add-ons Component: GUI engine Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants