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

[alternative] add logic to wait for the startup window to finish before showing information dialogs during startup #7150

Merged
merged 2 commits into from
May 25, 2015

Conversation

Montellese
Copy link
Member

This is an alternative (and IMO cleaner) approach to #6666.

This is an attempt at solving the problem that when we try to show a modal dialog during startup to provide the user with some information, the dialog isn't really shown to the user most of the time because by the time the dialog becomes visible, Confluence's Startup.xml kicks in and executes a ReplaceWindow() call to go to the home window. That ReplaceWindow() call changes the active window and basically hides the modal dialog in the background.

The difference to #6666 is that this doesn't add an ugly busy loop on the window manager to know when the user interface is really ready for these dialogs but instead uses a new GUI message GUI_MSG_UI_READY. That message is sent out in one of these cases:

  • Kodi has started without the login window and the startup window is not CGUIWindowStartup / Startup.xml
  • Kodi has started with the login window, the user has logged into one of the profiles and the startup window is not CGUIWindowStartup / Startup.xml
  • Kodi has started (with or without the login window) and the startup window is CGUIWindowStartup / Startup.xml and has been closed

@xhaggi
Copy link
Member

xhaggi commented May 17, 2015

looks good +1 from my side, thanks :)

@mkortstiege
Copy link
Member

+1

@xhaggi
Copy link
Member

xhaggi commented May 18, 2015

@Montellese we missed the case if the startup window is a PVR window. In that case we delegate the window activation to the PVR manager. See https://github.com/xbmc/xbmc/blob/master/xbmc/pvr/PVRManager.cpp#L518
This is done because we need to startup the PVR manager before we can activate a PVR related window.

For J*** we need to reconsider it because IMO we should not block the window activation. A better solution could be showing the progress dialog until the PVR manager is ready and the user can cancel the progress dialog and is returned to the previous window e.g. Home.

@Montellese
Copy link
Member Author

@xhaggi: I saw that PVR windows are handled special but I didn't know why and I didn't know that it was asynchronous. Could you take a look at the necessary changes since I can't really test it. I'm assuming that the message should be sent from that if block you linked to.

@xhaggi
Copy link
Member

xhaggi commented May 18, 2015

I'm assuming that the message should be sent from that if block you linked to.

yes

Could you take a look at the necessary changes since I can't really test it.

sure. if you want to test it by your-self you need to install the pvr.demo addon and set the startup window to a PVR related window.

…e migrated userdata once the user interface is really ready for usage
@Montellese
Copy link
Member Author

@xhaggi: Added handling of PVR startup window and tested it with pvr.demo. Could you take a look?
jenkins build this please

@xhaggi
Copy link
Member

xhaggi commented May 25, 2015

looks good, thanks

@MartijnKaijser
Copy link
Member

failure unrelated

@Montellese
Copy link
Member Author

OK to merge for beta?

@MartijnKaijser
Copy link
Member

i certainly won't object fixes :)

Montellese added a commit that referenced this pull request May 25, 2015
add logic to wait for the startup window to finish before showing information dialogs during startup
@Montellese Montellese merged commit c4cfa88 into xbmc:master May 25, 2015
@Montellese Montellese deleted the fix_startup_dialogs branch May 25, 2015 17:17
@MilhouseVH
Copy link
Contributor

Could someone confirm if this post from the forum is expected behaviour?

I'm not familiar with the <locale> settings in as.xml, or why <language>English</language> would be considered incorrect (and thus fail to load), which presumably is then causing the dialog to be successfully displayed thanks to the second commit in this PR.

Without this PR, there is no dialog shown, just a beep at startup, and the following WARNING output to kodi.log.

18:39:18  27.113977 T:1967521792 WARNING: CLangInfo: unable to load language "English". Trying to determine matching language addon...

With the PR, in addition to the above, we also now get the "Failed to load language" dialog.

Maybe the error isn't actually this PR, which is now correctly showing dialogs when required at startup, but rather the way this <language> tag is being (incorrectly?) handled, and which would be (almost) silently ignored in the past?

@Montellese
Copy link
Member Author

Well first of all the logic in CLangInfo should be capable of detecting that English is the resource.language.en_gb language addon and not fall back to it because it couldn't find one (but I thought there was a log message for that case too).

Secondly having <language>English</language> in advancedsettings.xml will lead to a situation where guisettings.xml will already contain the updated value of <language>resource.language.en_gb</language> but it will always be overwritten by the value from advancedsettings.xml so the startup logic will always have to try to determine the proper language addon and setting's value.

I've taken a quick look and determined the issue and it's specific to English in combination with advancedsettings.xml. I'll provide a fix ASAP.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants