From df7eec6bcb0b9fdee3888c9b4683dbb533eba300 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Wed, 3 Apr 2019 13:34:35 +0200 Subject: Windows QPA/File dialog: Refactor code copying non-file shell items Remove the canCopy() check which is not required and pass up the error message. Remove special characters and use the base name of the display name which can be an URL. Task-number: QTBUG-71785 Change-Id: I22966cb8d1f5bca0bbca71cf3ebe66e4ede1a747 Reviewed-by: Oliver Wolff Reviewed-by: Andre de la Rocha --- .../platforms/windows/qwindowsdialoghelpers.cpp | 88 ++++++++++++++++------ 1 file changed, 63 insertions(+), 25 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/platforms/windows/qwindowsdialoghelpers.cpp b/src/plugins/platforms/windows/qwindowsdialoghelpers.cpp index 1b17759b5f..ba441a1921 100644 --- a/src/plugins/platforms/windows/qwindowsdialoghelpers.cpp +++ b/src/plugins/platforms/windows/qwindowsdialoghelpers.cpp @@ -569,12 +569,10 @@ public: bool isFileSystem() const { return (m_attributes & SFGAO_FILESYSTEM) != 0; } bool isDir() const { return (m_attributes & SFGAO_FOLDER) != 0; } - // Copy using IFileOperation - bool canCopy() const { return (m_attributes & SFGAO_CANCOPY) != 0; } // Supports IStream bool canStream() const { return (m_attributes & SFGAO_STREAM) != 0; } - bool copyData(QIODevice *out); + bool copyData(QIODevice *out, QString *errorMessage); static IShellItems itemsFromItemArray(IShellItemArray *items); @@ -666,14 +664,19 @@ QWindowsShellItem::IShellItems QWindowsShellItem::itemsFromItemArray(IShellItemA return result; } -bool QWindowsShellItem::copyData(QIODevice *out) +bool QWindowsShellItem::copyData(QIODevice *out, QString *errorMessage) { - if (!canCopy() || !canStream()) + if (!canStream()) { + *errorMessage = QLatin1String("Item not streamable"); return false; + } IStream *istream = nullptr; HRESULT hr = m_item->BindToHandler(nullptr, BHID_Stream, IID_PPV_ARGS(&istream)); - if (FAILED(hr)) + if (FAILED(hr)) { + *errorMessage = QLatin1String("BindToHandler() failed: ") + + QLatin1String(QWindowsContext::comErrorString(hr)); return false; + } enum : ULONG { bufSize = 102400 }; char buffer[bufSize]; ULONG bytesRead; @@ -686,7 +689,12 @@ bool QWindowsShellItem::copyData(QIODevice *out) break; } istream->Release(); - return hr == S_OK || hr == S_FALSE; + if (hr != S_OK && hr != S_FALSE) { + *errorMessage = QLatin1String("Read() failed: ") + + QLatin1String(QWindowsContext::comErrorString(hr)); + return false; + } + return true; } // Helper for "Libraries": collections of folders appearing from Windows 7 @@ -735,8 +743,6 @@ void QWindowsShellItem::format(QDebug &d) const d << " [dir]"; if (canStream()) d << " [stream]"; - if (canCopy()) - d << " [copyable]"; d << ", normalDisplay=\"" << normalDisplay() << "\", desktopAbsoluteParsing=\"" << desktopAbsoluteParsing() << "\", urlString=\"" << urlString() << "\", fileSysPath=\"" << fileSysPath() << '"'; @@ -1395,21 +1401,50 @@ static void cleanupTemporaryItemCopies() QFile::remove(file); } -static QString createTemporaryItemCopy(QWindowsShellItem &qItem) +// Determine temporary file pattern from a shell item's display +// name. This can be a URL. + +static bool validFileNameCharacter(QChar c) { - if (!qItem.canCopy() || !qItem.canStream()) + return c.isLetterOrNumber() || c == QLatin1Char('_') || c == QLatin1Char('-'); +} + +QString tempFilePattern(QString name) +{ + const int lastSlash = qMax(name.lastIndexOf(QLatin1Char('/')), + name.lastIndexOf(QLatin1Char('\\'))); + if (lastSlash != -1) + name.remove(0, lastSlash + 1); + + int lastDot = name.lastIndexOf(QLatin1Char('.')); + if (lastDot < 0) + lastDot = name.size(); + name.insert(lastDot, QStringLiteral("_XXXXXX")); + + for (int i = lastDot - 1; i >= 0; --i) { + if (!validFileNameCharacter(name.at(i))) + name[i] = QLatin1Char('_'); + } + + name.prepend(QDir::tempPath() + QLatin1Char('/')); + return name; +} + +static QString createTemporaryItemCopy(QWindowsShellItem &qItem, QString *errorMessage) +{ + if (!qItem.canStream()) { + *errorMessage = QLatin1String("Item not streamable"); return QString(); - QString pattern = qItem.normalDisplay(); - const int lastDot = pattern.lastIndexOf(QLatin1Char('.')); - const QString placeHolder = QStringLiteral("_XXXXXX"); - if (lastDot >= 0) - pattern.insert(lastDot, placeHolder); - else - pattern.append(placeHolder); - - QTemporaryFile targetFile(QDir::tempPath() + QLatin1Char('/') + pattern); + } + + QTemporaryFile targetFile(tempFilePattern(qItem.normalDisplay())); targetFile.setAutoRemove(false); - if (!targetFile.open() || !qItem.copyData(&targetFile)) + if (!targetFile.open()) { + *errorMessage = QLatin1String("Cannot create temporary file: ") + + targetFile.errorString(); + return QString(); + } + if (!qItem.copyData(&targetFile, errorMessage)) return QString(); const QString result = targetFile.fileName(); if (temporaryItemCopies()->isEmpty()) @@ -1427,11 +1462,14 @@ QList QWindowsNativeOpenFileDialog::dialogResult() const QWindowsShellItem qItem(item); const QString path = qItem.path(); if (path.isEmpty() && !qItem.isDir()) { - const QString temporaryCopy = createTemporaryItemCopy(qItem); - if (temporaryCopy.isEmpty()) - qWarning() << "Unable to create a local copy of" << qItem; - else + QString errorMessage; + const QString temporaryCopy = createTemporaryItemCopy(qItem, &errorMessage); + if (temporaryCopy.isEmpty()) { + qWarning().noquote() << "Unable to create a local copy of" << qItem + << ": " << errorMessage; + } else { result.append(QUrl::fromLocalFile(temporaryCopy)); + } } else { result.append(qItem.url()); } -- cgit v1.2.3