From 4a7771f206d4b29be549d3827c36a46679d90de6 Mon Sep 17 00:00:00 2001 From: Eike Hein Date: Sun, 7 Jan 2018 13:02:01 +0900 Subject: [PATCH 1/4] QSimpleDrag: Fix mouse release coords for delayed event transmission On platforms such as XCB, the drag cursor pixmap is shown via a window (a QShapedPixmapWindow) under the cursor. The mouse button release event at the end of the drag is received in this QXcbWindow, but intercepted by an event filter that QSimpleDrag installs on the QApplication. It then resends it unmodified(!) after the drag has ended and the drag pixmap window destroyed, causing it to be delivered to the new top-level window. The local coordinates in the unmodified QMouseEvent are local to the drag pixmap window and don't match the window it is delayed-transmitted to. This ends up having fatal, user-visible effects particularly in Qt Quick: QQuickWindow synthesizes a hover event once per frame using the last received mouse coordinates, here: the release posted by QSimpleDrag. This is done to update the hover event state for items under the cursor when the mouse hasn't moved (e.g. QQuickMouseArea:: containsMouse). The bogus event coordinates in the release event then usually end up causing an item near the top-left of the QQuickWindow to assume it is hovered (because drag pixmap windows tend to be small), even when the mouse cursor is actually far away from it at the end of the drag. This shows up e.g. in the Plasma 5 desktop, where dragging an icon on the desktop will cause the icon at the top-left of the screen (if any) to switch to hovered state, as the release coordinates on the drag pixmap window (showing a dragged icon) fall into the geometry of the top-left icon. QSimpleDrag contains a topLevelAt() function to find the top-level window under the global cursor coordinates that is not the drag pixmap window. This is used by the drop event delivery code. This patch uses this function to find the relevant top-level window, then asks it to map the global cusor coordinates to its local coordinate system, then synthesizes a new QMouseEvent with local coordinates computed in this fashion. As a result the window now gets a release event with coordinates that make sense and are correct. Task-number: QTBUG-66103 Change-Id: I04ebe6ccd4a991fdd4b540ff0227973ea8896a9d Reviewed-by: Eike Hein Reviewed-by: Shawn Rutledge --- src/gui/kernel/qsimpledrag.cpp | 32 +++++++++++++++++++++++++++----- src/gui/kernel/qsimpledrag_p.h | 6 +++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/gui/kernel/qsimpledrag.cpp b/src/gui/kernel/qsimpledrag.cpp index a1e25dc53c..87d3ba5915 100644 --- a/src/gui/kernel/qsimpledrag.cpp +++ b/src/gui/kernel/qsimpledrag.cpp @@ -58,6 +58,7 @@ #include #include +#include #include #include @@ -69,6 +70,8 @@ QT_BEGIN_NAMESPACE #ifndef QT_NO_DRAGANDDROP +Q_LOGGING_CATEGORY(lcDnd, "qt.gui.dnd") + static QWindow* topLevelAt(const QPoint &pos) { QWindowList list = QGuiApplication::topLevelWindows(); @@ -94,10 +97,10 @@ static QWindow* topLevelAt(const QPoint &pos) */ QBasicDrag::QBasicDrag() : - m_restoreCursor(false), m_eventLoop(0), + m_current_window(nullptr), m_restoreCursor(false), m_eventLoop(nullptr), m_executed_drop_action(Qt::IgnoreAction), m_can_drop(false), - m_drag(0), m_drag_icon_window(0), m_useCompositing(true), - m_screen(Q_NULLPTR) + m_drag(nullptr), m_drag_icon_window(nullptr), m_useCompositing(true), + m_screen(nullptr) { } @@ -161,6 +164,7 @@ bool QBasicDrag::eventFilter(QObject *o, QEvent *e) return true; // Eat all mouse move events } case QEvent::MouseButtonRelease: + { disableEventFilter(); if (canDrop()) { QPoint nativePosition = getNativeMousePos(e, m_drag_icon_window); @@ -169,8 +173,25 @@ bool QBasicDrag::eventFilter(QObject *o, QEvent *e) cancel(); } exitDndEventLoop(); - QCoreApplication::postEvent(o, new QMouseEvent(*static_cast(e))); + + // If a QShapedPixmapWindow (drag feedback) is being dragged along, the + // mouse event's localPos() will be relative to that, which is useless. + // We want a position relative to the window where the drag ends, if possible (?). + // If there is no such window (belonging to this Qt application), + // make the event relative to the window where the drag started. (QTBUG-66103) + const QMouseEvent *release = static_cast(e); + const QWindow *releaseWindow = topLevelAt(release->globalPos()); + qCDebug(lcDnd) << "mouse released over" << releaseWindow << "after drag from" << m_current_window << "globalPos" << release->globalPos(); + if (!releaseWindow) + releaseWindow = m_current_window; + QPoint releaseWindowPos = (releaseWindow ? releaseWindow->mapFromGlobal(release->globalPos()) : release->globalPos()); + QMouseEvent *newRelease = new QMouseEvent(release->type(), + releaseWindowPos, releaseWindowPos, release->screenPos(), + release->button(), release->buttons(), + release->modifiers(), release->source()); + QCoreApplication::postEvent(o, newRelease); return true; // defer mouse release events until drag event loop has returned + } case QEvent::MouseButtonDblClick: case QEvent::Wheel: return true; @@ -349,7 +370,7 @@ static inline QPoint fromNativeGlobalPixels(const QPoint &point) into account. */ -QSimpleDrag::QSimpleDrag() : m_current_window(0) +QSimpleDrag::QSimpleDrag() { } @@ -373,6 +394,7 @@ void QSimpleDrag::startDrag() updateCursor(Qt::IgnoreAction); } setExecutedDropAction(Qt::IgnoreAction); + qCDebug(lcDnd) << "drag began from" << m_current_window<< "cursor pos" << QCursor::pos() << "can drop?" << canDrop(); } void QSimpleDrag::cancel() diff --git a/src/gui/kernel/qsimpledrag_p.h b/src/gui/kernel/qsimpledrag_p.h index 0b8a0bc703..bbd7f7f4bb 100644 --- a/src/gui/kernel/qsimpledrag_p.h +++ b/src/gui/kernel/qsimpledrag_p.h @@ -105,6 +105,9 @@ protected: QDrag *drag() const { return m_drag; } +protected: + QWindow *m_current_window; + private: void enableEventFilter(); void disableEventFilter(); @@ -132,9 +135,6 @@ protected: virtual void cancel() Q_DECL_OVERRIDE; virtual void move(const QPoint &globalPos) Q_DECL_OVERRIDE; virtual void drop(const QPoint &globalPos) Q_DECL_OVERRIDE; - -private: - QWindow *m_current_window; }; #endif // QT_NO_DRAGANDDROP From 4cd90a3579986ae7441c3e982a5f34cdfd92c152 Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Tue, 17 Jan 2017 22:31:04 +0000 Subject: [PATCH 2/4] Fix native QFileDialog initial selection for remote URLs When using QFileDialog::getOpenFileUrl() with dir set to e.g. "sftp://foo/bar.cpp" we call q->selectDirectoryUrl("sftp://foo") followed by q->selectFile("bar.cpp"). Inside QFileDialog::selectFile() we unconditionally convert "bar.cpp" to an absolute URL and then call d->selectFile_sys("$CWD/bar.cpp") This then calls platform integration that detects that an absolute URL is being passed to selectFile() and therefore overrides the initial directory. Initially reported as https://bugs.kde.org/show_bug.cgi?id=374913 This is a regression that appeared some time between Qt 5.7.0 and 5.7.1. I have not had time to bisect this but the only commit that may have change behavior in that time range appears to be 007f92c6eef6191c48da0c44916591d48813ae62. Change-Id: I6968abe9ed5c5b9de067c453a7e9d2c5cdb3a190 Reviewed-by: Christoph Resch Reviewed-by: David Faure --- src/widgets/dialogs/qfiledialog.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/widgets/dialogs/qfiledialog.cpp b/src/widgets/dialogs/qfiledialog.cpp index 8d37969be4..cb2c534b24 100644 --- a/src/widgets/dialogs/qfiledialog.cpp +++ b/src/widgets/dialogs/qfiledialog.cpp @@ -2830,7 +2830,10 @@ void QFileDialogPrivate::init(const QUrl &directory, const QString &nameFilter, if (!nameFilter.isEmpty()) q->setNameFilter(nameFilter); q->setDirectoryUrl(workingDirectory(directory)); - q->selectFile(initialSelection(directory)); + if (directory.isLocalFile()) + q->selectFile(initialSelection(directory)); + else + q->selectUrl(directory); #ifndef QT_NO_SETTINGS // Try to restore from the FileDialog settings group; if it fails, fall back From 9b2913377807b3dae107befeec0bc488f4a2cbad Mon Sep 17 00:00:00 2001 From: Nathan Collins Date: Mon, 29 Jan 2018 14:37:57 +0000 Subject: [PATCH 3/4] Fix QFileDialog::defaultSuffix on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't prepend the default suffix to the NSSavePanel allowedFileTypes. "If no extension is given by the user, the first item in the allowedFileTypes array will be used as the extension for the save panel." The user expects to get the suffix displayed to them in the drop down filter, not the default suffix set by the developer. Apply the default suffix if neither the user or the NSSavePanel provide a suffix. Task-number: QTBUG-66066 Change-Id: I64093b9f3178bd2377a7b65d6f23aed6214a4119 Reviewed-by: Morten Johan Sørvig --- .../platforms/cocoa/qcocoafiledialoghelper.mm | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm b/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm index 74148b7cbf..c1c7903766 100644 --- a/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm +++ b/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm @@ -418,6 +418,13 @@ static QString strippedText(QString s) } else { QList result; QString filename = QString::fromNSString([[mSavePanel URL] path]).normalized(QString::NormalizationForm_C); + const QString defaultSuffix = mOptions->defaultSuffix(); + const QFileInfo fileInfo(filename); + // If neither the user or the NSSavePanel have provided a suffix, use + // the default suffix (if it exists). + if (fileInfo.suffix().isEmpty() && !defaultSuffix.isEmpty()) { + filename.append('.').append(defaultSuffix); + } result << QUrl::fromLocalFile(filename.remove(QLatin1String("___qt_very_unlikely_prefix_"))); return result; } @@ -445,10 +452,7 @@ static QString strippedText(QString s) [mPopUpButton setHidden:chooseDirsOnly]; // TODO hide the whole sunken pane instead? if (mOptions->acceptMode() == QFileDialogOptions::AcceptSave) { - QStringList ext = [self acceptableExtensionsForSave]; - const QString defaultSuffix = mOptions->defaultSuffix(); - if (!ext.isEmpty() && !defaultSuffix.isEmpty()) - ext.prepend(defaultSuffix); + const QStringList ext = [self acceptableExtensionsForSave]; [mSavePanel setAllowedFileTypes:ext.isEmpty() ? nil : qt_mac_QStringListToNSMutableArray(ext)]; } else { [mOpenPanel setAllowedFileTypes:nil]; // delegate panel:shouldEnableURL: does the file filtering for NSOpenPanel From b44df9937e4b15596b994f8e20822b83ac4bed29 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Tue, 30 Jan 2018 21:02:10 +0100 Subject: [PATCH 4/4] Clean-up in QDateTime's parsing of ISODate{,WithMs} Actually check that there's a T where ISO 8601 wants it (instead of just skipping over whatever's there), with something after it; move some declarations later; add some comments; and use the QStringRef API more cleanly (so that it's easier to see what's going on). Simplify a loop condition to avoid the need for a post-loop fix-up. This incidentally prevents an assertion failure (which brought the mess to my attention) parsing a short string as an ISO date-time; if there's a T with nothing after it, we won't try to read at index -1 in the following text. (The actual fail seen had a Z where the T should have been, with nothing after it.) Add tests for invalid ISOdate cases that triggered the assertion. Task-number: QTBUG-66076 Change-Id: Ided9adf62a56d98f144bdf91b40f918e22bd82cd Reviewed-by: Israel Lins Albuquerque Reviewed-by: Thiago Macieira (cherry picked from commit a9c111ed8c30a5a8fec3f02244f0d5a4bd08e931) --- src/corelib/tools/qdatetime.cpp | 23 +++++++++++++------ .../corelib/tools/qdatetime/tst_qdatetime.cpp | 4 ++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/corelib/tools/qdatetime.cpp b/src/corelib/tools/qdatetime.cpp index 9d26207a0f..045ad10755 100644 --- a/src/corelib/tools/qdatetime.cpp +++ b/src/corelib/tools/qdatetime.cpp @@ -4720,25 +4720,35 @@ QDateTime QDateTime::fromString(const QString& string, Qt::DateFormat format) if (size < 10) return QDateTime(); - QStringRef isoString(&string); - Qt::TimeSpec spec = Qt::LocalTime; - QDate date = QDate::fromString(string.left(10), Qt::ISODate); if (!date.isValid()) return QDateTime(); if (size == 10) return QDateTime(date); - isoString = isoString.right(isoString.length() - 11); + Qt::TimeSpec spec = Qt::LocalTime; + QStringRef isoString(&string); + isoString = isoString.mid(10); // trim "yyyy-MM-dd" + + // Must be left with T and at least one digit for the hour: + if (isoString.size() < 2 + || !(isoString.startsWith(QLatin1Char('T')) + // FIXME: QSql relies on QVariant::toDateTime() accepting a space here: + || isoString.startsWith(QLatin1Char(' ')))) { + return QDateTime(); + } + isoString = isoString.mid(1); // trim 'T' (or space) + int offset = 0; // Check end of string for Time Zone definition, either Z for UTC or [+-]HH:mm for Offset if (isoString.endsWith(QLatin1Char('Z'))) { spec = Qt::UTC; - isoString = isoString.left(isoString.size() - 1); + isoString.chop(1); // trim 'Z' } else { // the loop below is faster but functionally equal to: // const int signIndex = isoString.indexOf(QRegExp(QStringLiteral("[+-]"))); int signIndex = isoString.size() - 1; + Q_ASSERT(signIndex >= 0); bool found = false; { const QChar plus = QLatin1Char('+'); @@ -4746,8 +4756,7 @@ QDateTime QDateTime::fromString(const QString& string, Qt::DateFormat format) do { QChar character(isoString.at(signIndex)); found = character == plus || character == minus; - } while (--signIndex >= 0 && !found); - ++signIndex; + } while (!found && --signIndex >= 0); } if (found) { diff --git a/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp b/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp index 28ad2d193c..91b5966fa8 100644 --- a/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp +++ b/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp @@ -2277,6 +2277,10 @@ void tst_QDateTime::fromStringDateFormat_data() QTest::newRow("ISO .99999 of a minute (comma)") << QString::fromLatin1("2012-01-01T08:00,99999") << Qt::ISODate << QDateTime(QDate(2012, 1, 1), QTime(8, 0, 59, 999), Qt::LocalTime); QTest::newRow("ISO empty") << QString::fromLatin1("") << Qt::ISODate << invalidDateTime(); + QTest::newRow("ISO short") << QString::fromLatin1("2017-07-01T") << Qt::ISODate << invalidDateTime(); + QTest::newRow("ISO zoned date") << QString::fromLatin1("2017-07-01Z") << Qt::ISODate << invalidDateTime(); + QTest::newRow("ISO zoned empty time") << QString::fromLatin1("2017-07-01TZ") << Qt::ISODate << invalidDateTime(); + QTest::newRow("ISO mis-punctuated") << QString::fromLatin1("2018/01/30 ") << Qt::ISODate << invalidDateTime(); // Test Qt::RFC2822Date format (RFC 2822). QTest::newRow("RFC 2822 +0100") << QString::fromLatin1("13 Feb 1987 13:24:51 +0100")