Cocoa: Make child window cursors work correctly

The existing cursor logic had a couple of issues:

- It made the faulty assumption that we could not use
the NSWindow invalidateCursorRectsForView API for
child NSViews.

- It used NSWindow invalidateCursorRectsForView and
NSView resetCursorRects. This API has been replaced
by the more general NSTrackingArea API.

- It did not implement falling back to the parent
window cursor if the current window has no cursor
set.

Document that QWindow cursors work the same way as
QWidget cursors in that a QWindow with no set cursor
will fall back to the parent window cursor.

Change the cocoa platform code to use NSTrackingArea
exclusively and implement NSView cursorUpdate which
sets the cursor. Handle immediate change on QWindow::
setCursor() manually.

Add QWindow::effectiveWindowCursor() and
applyEffectiveWindowCursor() which finds the correct
window cursor.

Add a manual test for the child window, child widget,
and QWidget::createWindowChild cases.

Task-number: QTBUG-33479
Task-number: QTBUG-52023
Change-Id: I0370e11bbadb2da95e8632e61be6228ec2cd5e9d
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>

13 files changed, 395 insertions, 39 deletions

diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp
index 2ff19f5175..c7ad10a46f 100644
--- a/src/gui/kernel/qwindow.cpp
+++ b/src/gui/kernel/qwindow.cpp
@@ -2462,6 +2462,9 @@ void QWindowPrivate::_q_clearAlert()
     See the \l{Qt::CursorShape}{list of predefined cursor objects} for a
     range of useful shapes.
 
+    If no cursor has been set, or after a call to unsetCursor(), the
+    parent window's cursor is used.
+
     By default, the cursor has the Qt::ArrowCursor shape.
 
     Some underlying window implementations will reset the cursor if it
diff --git a/src/plugins/platforms/cocoa/qcocoacursor.mm b/src/plugins/platforms/cocoa/qcocoacursor.mm
index 8e38181c29..a4c291c14a 100644
--- a/src/plugins/platforms/cocoa/qcocoacursor.mm
+++ b/src/plugins/platforms/cocoa/qcocoacursor.mm
@@ -55,7 +55,7 @@ QCocoaCursor::~QCocoaCursor()
 
 void QCocoaCursor::changeCursor(QCursor *cursor, QWindow *window)
 {
-    NSCursor * cocoaCursor = convertCursor(cursor);
+    NSCursor *cocoaCursor = convertCursor(cursor);
 
     if (QPlatformWindow * platformWindow = window->handle())
         static_cast<QCocoaWindow *>(platformWindow)->setWindowCursor(cocoaCursor);
@@ -77,9 +77,12 @@ void QCocoaCursor::setPos(const QPoint &position)
     CFRelease(e);
 }
 
-NSCursor *QCocoaCursor::convertCursor(QCursor * cursor)
+NSCursor *QCocoaCursor::convertCursor(QCursor *cursor)
 {
-    const Qt::CursorShape newShape = cursor ? cursor->shape() : Qt::ArrowCursor;
+    if (cursor == Q_NULLPTR)
+        return 0;
+
+    const Qt::CursorShape newShape = cursor->shape();
     NSCursor *cocoaCursor;
 
     // Check for a suitable built-in NSCursor first:
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.h b/src/plugins/platforms/cocoa/qcocoawindow.h
index 9cf6328281..bf28f83540 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.h
+++ b/src/plugins/platforms/cocoa/qcocoawindow.h
@@ -236,6 +236,8 @@ public:
     void setMenubar(QCocoaMenuBar *mb);
     QCocoaMenuBar *menubar() const;
 
+    NSCursor *effectiveWindowCursor() const;
+    void applyEffectiveWindowCursor();
     void setWindowCursor(NSCursor *cursor);
 
     void registerTouch(bool enable);
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 977a5ae657..a18d93b89e 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -1645,31 +1645,53 @@ QCocoaMenuBar *QCocoaWindow::menubar() const
     return m_menubar;
 }
 
-void QCocoaWindow::setWindowCursor(NSCursor *cursor)
+// Finds the effective cursor for this window by walking up the
+// ancestor chain (including this window) until a set cursor is
+// found. Returns nil if there is not set cursor.
+NSCursor *QCocoaWindow::effectiveWindowCursor() const
 {
-    // This function is called (via QCocoaCursor) by Qt to set
-    // the cursor for this window. It can be called for a window
-    // that is not currenly under the mouse pointer (for example
-    // for a popup window.) Qt expects the set cursor to "stick":
-    // it should be accociated with the window until a different
-    // cursor is set.
-    if (m_windowCursor != cursor) {
-        [m_windowCursor release];
-        m_windowCursor = [cursor retain];
-    }
 
-    // Use the built in cursor rect API if the QCocoaWindow has a NSWindow.
-    // Othervise, set the cursor if this window is under the mouse. In
-    // this case QNSView::cursorUpdate will set the cursor as the pointer
-    // moves.
-    if (m_nsWindow && m_qtView) {
-        [m_nsWindow invalidateCursorRectsForView : m_qtView];
+    if (m_windowCursor)
+        return m_windowCursor;
+    if (!parent())
+        return nil;
+    return static_cast<QCocoaWindow *>(parent())->effectiveWindowCursor();
+}
+
+// Applies the cursor as returned by effectiveWindowCursor(), handles
+// the special no-cursor-set case by setting the arrow cursor.
+void QCocoaWindow::applyEffectiveWindowCursor()
+{
+    NSCursor *effectiveCursor = effectiveWindowCursor();
+    if (effectiveCursor) {
+        [effectiveCursor set];
     } else {
-        if (m_windowUnderMouse)
-            [cursor set];
+        // We wold like to _unset_ the cursor here; but there is no such
+        // API. Fall back to setting the default arrow cursor.
+        [[NSCursor arrowCursor] set];
     }
 }
 
+void QCocoaWindow::setWindowCursor(NSCursor *cursor)
+{
+    if (m_windowCursor == cursor)
+        return;
+
+    // Setting a cursor in a foregin view is not supported.
+    if (!m_qtView)
+        return;
+
+    [m_windowCursor release];
+    m_windowCursor = cursor;
+    [m_windowCursor retain];
+
+    // The installed view tracking area (see QNSView updateTrackingAreas) will
+    // handle cursor updates on mouse enter/leave. Handle the case where the
+    // mouse is on the this window by changing the cursor immediately.
+    if (m_windowUnderMouse)
+        applyEffectiveWindowCursor();
+}
+
 void QCocoaWindow::registerTouch(bool enable)
 {
     m_registerTouchCount += enable ? 1 : -1;
diff --git a/src/plugins/platforms/cocoa/qnsview.h b/src/plugins/platforms/cocoa/qnsview.h
index 2d4ad7aad3..9d2b54a321 100644
--- a/src/plugins/platforms/cocoa/qnsview.h
+++ b/src/plugins/platforms/cocoa/qnsview.h
@@ -118,7 +118,6 @@ Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper));
 - (void)mouseMovedImpl:(NSEvent *)theEvent;
 - (void)mouseEnteredImpl:(NSEvent *)theEvent;
 - (void)mouseExitedImpl:(NSEvent *)theEvent;
-- (void)cursorUpdateImpl:(NSEvent *)theEvent;
 - (void)rightMouseDown:(NSEvent *)theEvent;
 - (void)rightMouseDragged:(NSEvent *)theEvent;
 - (void)rightMouseUp:(NSEvent *)theEvent;
diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm
index c67bcfd23b..1ad9b5f327 100644
--- a/src/plugins/platforms/cocoa/qnsview.mm
+++ b/src/plugins/platforms/cocoa/qnsview.mm
@@ -120,7 +120,7 @@ static bool _q_dontOverrideCtrlLMB = false;
 
 - (void)cursorUpdate:(NSEvent *)theEvent
 {
-    [view cursorUpdateImpl:theEvent];
+    [self cursorUpdate:theEvent];
 }
 
 @end
@@ -924,21 +924,10 @@ QT_WARNING_POP
     [self addTrackingArea:m_trackingArea];
 }
 
--(void)cursorUpdateImpl:(NSEvent *)theEvent
-{
-    Q_UNUSED(theEvent)
-    // Set the cursor manually if there is no NSWindow.
-    if (!m_platformWindow->m_nsWindow && m_platformWindow->m_windowCursor)
-        [m_platformWindow->m_windowCursor set];
-    else
-        [super cursorUpdate:theEvent];
-}
-
--(void)resetCursorRects
+- (void)cursorUpdate:(NSEvent *)theEvent
 {
-    // Use the cursor rect API if there is a NSWindow
-    if (m_platformWindow->m_nsWindow && m_platformWindow->m_windowCursor)
-        [self addCursorRect:[self visibleRect] cursor:m_platformWindow->m_windowCursor];
+    Q_UNUSED(theEvent);
+    m_platformWindow->applyEffectiveWindowCursor();
 }
 
 - (void)mouseMovedImpl:(NSEvent *)theEvent
diff --git a/tests/manual/qcursor/childwidget/childwidget.pro b/tests/manual/qcursor/childwidget/childwidget.pro
new file mode 100644
index 0000000000..9ca41c5b4f
--- /dev/null
+++ b/tests/manual/qcursor/childwidget/childwidget.pro
@@ -0,0 +1,6 @@
+TEMPLATE = app
+TARGET = childwidget
+INCLUDEPATH += .
+QT += widgets
+
+SOURCES += main.cpp
diff --git a/tests/manual/qcursor/childwidget/main.cpp b/tests/manual/qcursor/childwidget/main.cpp
new file mode 100644
index 0000000000..4447c87210
--- /dev/null
+++ b/tests/manual/qcursor/childwidget/main.cpp
@@ -0,0 +1,92 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+#include <QtWidgets>
+
+class CursorWidget : public QWidget
+{
+public:
+    CursorWidget(QCursor cursor, QColor color)
+    :m_cursor(cursor)
+    ,m_color(color)
+    {
+        if (cursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(cursor);
+    }
+
+    void paintEvent(QPaintEvent *e)
+    {
+        QPainter p(this);
+        p.fillRect(e->rect(), m_color);
+    }
+
+    void mousePressEvent(QMouseEvent *)
+    {
+        // Toggle cursor
+        QCursor newCursor = (cursor().shape() == m_cursor.shape()) ? QCursor() : m_cursor;
+        if (newCursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(newCursor);
+    }
+
+private:
+    QCursor m_cursor;
+    QColor m_color;
+};
+
+int main(int argc, char **argv)
+{
+    QApplication app(argc, argv);
+
+    // Test child widgets (one of which is native) with set cursors.
+    // Click window to toggle cursor.
+
+    CursorWidget w1((QCursor(Qt::SizeVerCursor)), QColor(Qt::blue).darker());
+    w1.resize(200, 200);
+    w1.show();
+
+    CursorWidget w2((QCursor(Qt::OpenHandCursor)), QColor(Qt::red).darker());
+    w2.setParent(&w1);
+    w2.setGeometry(0, 0, 100, 100);
+    w2.show();
+
+    CursorWidget w3((QCursor(Qt::IBeamCursor)), QColor(Qt::green).darker());
+    w3.winId();
+    w3.setParent(&w1);
+    w3.setGeometry(100, 100, 100, 100);
+    w3.show();
+
+    return app.exec();
+}
diff --git a/tests/manual/qcursor/childwindow/childwindow.pro b/tests/manual/qcursor/childwindow/childwindow.pro
new file mode 100644
index 0000000000..194536a91a
--- /dev/null
+++ b/tests/manual/qcursor/childwindow/childwindow.pro
@@ -0,0 +1,5 @@
+TEMPLATE = app
+TARGET = childwindow
+INCLUDEPATH += .
+
+SOURCES += main.cpp
diff --git a/tests/manual/qcursor/childwindow/main.cpp b/tests/manual/qcursor/childwindow/main.cpp
new file mode 100644
index 0000000000..5fc293dfcf
--- /dev/null
+++ b/tests/manual/qcursor/childwindow/main.cpp
@@ -0,0 +1,91 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+#include <QtGui>
+
+class CursorWindow : public QRasterWindow
+{
+public:
+    CursorWindow(QCursor cursor, QColor color)
+    :m_cursor(cursor)
+    ,m_color(color)
+    {
+        if (cursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(cursor);
+    }
+
+    void paintEvent(QPaintEvent *e)
+    {
+        QPainter p(this);
+        p.fillRect(e->rect(), m_color);
+    }
+
+    void mousePressEvent(QMouseEvent *)
+    {
+        // Toggle cursor
+        QCursor newCursor = (cursor().shape() == m_cursor.shape()) ? QCursor() : m_cursor;
+        if (newCursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(newCursor);
+    }
+
+private:
+    QCursor m_cursor;
+    QColor m_color;
+};
+
+int main(int argc, char **argv)
+{
+    QGuiApplication app(argc, argv);
+
+    // Test child windows with set cursors. Create parent window and
+    // two child windows. Click window to toggle cursor.
+
+    CursorWindow w1((QCursor(Qt::SizeVerCursor)), QColor(Qt::blue).darker());
+    w1.resize(200, 200);
+    w1.show();
+
+    CursorWindow w2((QCursor(Qt::OpenHandCursor)), QColor(Qt::red).darker());
+    w2.setParent(&w1);
+    w2.setGeometry(0, 0, 100, 100);
+    w2.show();
+
+    CursorWindow w3((QCursor(Qt::IBeamCursor)), QColor(Qt::green).darker());
+    w3.setParent(&w1);
+    w3.setGeometry(100, 100, 100, 100);
+    w3.show();
+
+    return app.exec();
+}
diff --git a/tests/manual/qcursor/childwindowcontainer/childwindowcontainer.pro b/tests/manual/qcursor/childwindowcontainer/childwindowcontainer.pro
new file mode 100644
index 0000000000..2233ce4a63
--- /dev/null
+++ b/tests/manual/qcursor/childwindowcontainer/childwindowcontainer.pro
@@ -0,0 +1,6 @@
+TEMPLATE = app
+TARGET = childwindowcontainer
+INCLUDEPATH += .
+QT += widgets
+
+SOURCES += main.cpp
diff --git a/tests/manual/qcursor/childwindowcontainer/main.cpp b/tests/manual/qcursor/childwindowcontainer/main.cpp
new file mode 100644
index 0000000000..d440133a42
--- /dev/null
+++ b/tests/manual/qcursor/childwindowcontainer/main.cpp
@@ -0,0 +1,138 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 The Qt Company Ltd.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+#include <QtWidgets>
+
+class CursorWindow : public QRasterWindow
+{
+public:
+    CursorWindow(QCursor cursor, QColor color)
+    :m_cursor(cursor)
+    ,m_color(color)
+    {
+        if (cursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(cursor);
+    }
+
+    void paintEvent(QPaintEvent *e)
+    {
+        QPainter p(this);
+        p.fillRect(e->rect(), m_color);
+    }
+
+    void mousePressEvent(QMouseEvent *)
+    {
+        // Toggle cursor
+        QCursor newCursor = (cursor().shape() == m_cursor.shape()) ? QCursor() : m_cursor;
+        if (newCursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(newCursor);
+    }
+
+private:
+    QCursor m_cursor;
+    QColor m_color;
+};
+
+class CursorWidget : public QWidget
+{
+public:
+    CursorWidget(QCursor cursor, QColor color)
+    :m_cursor(cursor)
+    ,m_color(color)
+    {
+        if (cursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(cursor);
+    }
+
+    void paintEvent(QPaintEvent *e)
+    {
+        QPainter p(this);
+        p.fillRect(e->rect(), m_color);
+    }
+
+    void mousePressEvent(QMouseEvent *)
+    {
+        // Toggle cursor
+        QCursor newCursor = (cursor().shape() == m_cursor.shape()) ? QCursor() : m_cursor;
+        if (newCursor.shape() == Qt::ArrowCursor)
+            unsetCursor();
+        else
+            setCursor(newCursor);
+    }
+
+private:
+    QCursor m_cursor;
+    QColor m_color;
+};
+
+int main(int argc, char **argv)
+{
+    QApplication app(argc, argv);
+
+    {
+        // Create top-level windowContainer with window. Setting the cursor
+        // for the container should set the cursor for the window as well.
+        // Setting the cursor for the window overrides the cursor for the
+        // container. The example starts out with a window cursor; click
+        // to fall back to the container cursor.
+        CursorWindow *w1 = new CursorWindow(QCursor(Qt::OpenHandCursor), QColor(Qt::red).darker());
+        QWidget* container = QWidget::createWindowContainer(w1);
+        container->resize(200, 200);
+        container->setCursor(Qt::PointingHandCursor);
+        container->show();
+    }
+
+    {
+        // Similar to above, but with a top-level QWiget
+        CursorWidget *w1 = new CursorWidget(QCursor(Qt::IBeamCursor), QColor(Qt::green).darker());
+        w1->resize(200, 200);
+
+        CursorWindow *w2 = new CursorWindow(QCursor(Qt::OpenHandCursor), QColor(Qt::red).darker());
+        QWidget* container = QWidget::createWindowContainer(w2);
+        container->winId(); // must make the container native, otherwise setCursor
+                            // sets the cursor on a QWindowContainerClassWindow which
+                            // is outside the QWindow hierarchy (macOS).
+        container->setParent(w1);
+        container->setCursor(Qt::PointingHandCursor);
+        container->setGeometry(0, 0, 100, 100);
+
+        w1->show();
+    }
+
+    return app.exec();
+}
diff --git a/tests/manual/qcursor/qcursor.pro b/tests/manual/qcursor/qcursor.pro
index 0b5c2b1945..c6617b8e89 100644
--- a/tests/manual/qcursor/qcursor.pro
+++ b/tests/manual/qcursor/qcursor.pro
@@ -1,3 +1,3 @@
 TEMPLATE = subdirs
 
-SUBDIRS = allcursors grab_override qcursorhighdpi
+SUBDIRS = allcursors childwidget childwindow childwindowcontainer grab_override qcursorhighdpi