Skip to content

Loading…

Filemanager fixes for guimediawindow too #1997

Merged
merged 2 commits into from

3 participants

@arnova
Team Kodi member

This is followup implements the same fixes for guimediawindow as already done in the filemanager class. @jmarshallnz: Please review.

@jmarshallnz
Team Kodi member
@arnova
Team Kodi member

@jmarshallnz : Can this be pulled in now?

@jmarshallnz
Team Kodi member

Not until Frodo is out.

@MartijnKaijser
Team Kodi member

@arnova
Good to go?

@arnova
Team Kodi member

Yup unless @jmarshallnz says no ;-)

@jmarshallnz
Team Kodi member

Under what cases do you hit this? It changes behaviour as to always throw a dialog when GetDirectory() returns false.

Why is the change in Refresh() needed exactly?

@arnova
Team Kodi member

Well my reasoning was that instead of silenty returning to root when GetDirectory() fails, with the chance of the user not noticing it, the user always gets an error dialog presented (and since the dialog text depends on the item's path, you need to pass it thus the change in Refresh()). I can remove this behavior, if you like, I don't particularly prefer one... ;-)

@arnova
Team Kodi member
@jmarshallnz
Team Kodi member

The reason I don't like the dialog in Refresh() is that it may be called without user interaction. Thus, throwing a dialog up on failure may not be the most appropriate thing to do. Better to just fail silently in that case.

Ofcourse, I'm assuming that you haven't actually hit a case where the dialog would be needed in Refresh.

@arnova
Team Kodi member

@jmarshallnz : The case I hit where I thought a dialog could be nice is when a user browses to a usb stick, goes to the homescreen, janks out the usb stick, and returns to the media-window. But agreed, I think we shouldn't have it in Refresh() so I removed it there. In case you think the rest is ok, go ahead and merge.

@jmarshallnz jmarshallnz merged commit c1fc380 into xbmc:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 28, 2012
  1. changed: Refactor CGUIMediaWindow::ShowShareErrorMessage() so it work…

    arnova committed
    …s with any path
Commits on Feb 20, 2013
  1. fixed: Properly fallback to root in case getdirectory fails in Update…

    arnova committed
    …() + show error dialog (in line with a54cb0d)
Showing with 17 additions and 23 deletions.
  1. +17 −23 xbmc/windows/GUIMediaWindow.cpp
View
40 xbmc/windows/GUIMediaWindow.cpp
@@ -777,16 +777,13 @@ bool CGUIMediaWindow::Update(const CStdString &strDirectory, bool updateFilterPa
if (!GetDirectory(directory, items))
{
CLog::Log(LOGERROR,"CGUIMediaWindow::GetDirectory(%s) failed", strDirectory.c_str());
- // if the directory is the same as the old directory, then we'll return
- // false. Else, we assume we can get the previous directory
- if (strDirectory.Equals(strCurrentDirectory))
- return false;
+ // Try to return to the previous directory, if not the same
+ // else fallback to root
+ if (strDirectory.Equals(strCurrentDirectory) || !Update(m_history.RemoveParentPath()))
+ Update(""); // Fallback to root
- // We assume, we can get the parent
- // directory again, but we have to
- // return false to be able to eg. show
+ // Return false to be able to eg. show
// an error message.
- Update(m_history.RemoveParentPath());
return false;
}
@@ -1188,21 +1185,18 @@ bool CGUIMediaWindow::HaveDiscOrConnection(const CStdString& strPath, int iDrive
// \brief Shows a standard errormessage for a given pItem.
void CGUIMediaWindow::ShowShareErrorMessage(CFileItem* pItem)
{
- if (pItem->m_bIsShareOrDrive)
- {
- int idMessageText=0;
- const CURL& url=pItem->GetAsUrl();
- const CStdString& strHostName=url.GetHostName();
-
- if (pItem->m_iDriveType != CMediaSource::SOURCE_TYPE_REMOTE) // Local shares incl. dvd drive
- idMessageText=15300;
- else if (url.GetProtocol() == "smb" && strHostName.IsEmpty()) // smb workgroup
- idMessageText=15303;
- else // All other remote shares
- idMessageText=15301;
-
- CGUIDialogOK::ShowAndGetInput(220, idMessageText, 0, 0);
- }
+ int idMessageText = 0;
+ CURL url(pItem->GetPath());
+ const CStdString& strHostName = url.GetHostName();
+
+ if (url.GetProtocol() == "smb" && strHostName.IsEmpty()) // smb workgroup
+ idMessageText = 15303; // Workgroup not found
+ else if (pItem->m_iDriveType == CMediaSource::SOURCE_TYPE_REMOTE || URIUtils::IsRemote(pItem->GetPath()))
+ idMessageText = 15301; // Could not connect to network server
+ else
+ idMessageText = 15300; // Path not found or invalid
+
+ CGUIDialogOK::ShowAndGetInput(220, idMessageText, 0, 0);
}
// \brief The functon goes up one level in the directory tree
Something went wrong with that request. Please try again.