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

Qt: Don't crash when pressing the Return key #6099

Merged
merged 1 commit into from Oct 11, 2017

Conversation

4 participants
@leoetlino
Member

leoetlino commented Oct 4, 2017

Use the "activated" signal instead of manually detecting key presses.
Handles both double clicks and Return presses, and fixes a bug in the
logic which could cause GameSelected to be emitted without any game.

@spycrab

spycrab approved these changes Oct 4, 2017

@leoetlino leoetlino added this to the Qt milestone Oct 5, 2017

@ligfx

This comment has been minimized.

Show comment
Hide comment
@ligfx

ligfx Oct 7, 2017

Contributor

On macOS, this no longer handles Return presses (instead activated binds to +O, which is used in e.g. Finder).

Contributor

ligfx commented Oct 7, 2017

On macOS, this no longer handles Return presses (instead activated binds to +O, which is used in e.g. Finder).

@Helios747

This comment has been minimized.

Show comment
Hide comment
@Helios747

Helios747 Oct 7, 2017

Contributor

I love this bug.

Contributor

Helios747 commented Oct 7, 2017

I love this bug.

@leoetlino

This comment has been minimized.

Show comment
Hide comment
@leoetlino

leoetlino Oct 7, 2017

Member

Ugh, looks like Qt item views don't emit the activated signal at all on macOS...

https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/itemviews/qabstractitemview.cpp?h=5.5#n2389

#ifdef Q_OS_MAC
    case Qt::Key_Enter:
    case Qt::Key_Return:
        // Propagate the enter if you couldn't edit the item and there are no
        // current editors (if there are editors, the event was most likely propagated from it).
        if (!edit(currentIndex(), EditKeyPressed, event) && d->editorIndexHash.isEmpty())
            event->ignore();
        break;
#else
    case Qt::Key_Enter:
    case Qt::Key_Return:
        // ### we can't open the editor on enter, becuse
        // some widgets will forward the enter event back
        // to the viewport, starting an endless loop
        if (state() != EditingState || hasFocus()) {
            if (currentIndex().isValid())
                emit activated(currentIndex());
            event->ignore();
        }
        break;
#endif
Member

leoetlino commented Oct 7, 2017

Ugh, looks like Qt item views don't emit the activated signal at all on macOS...

https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/itemviews/qabstractitemview.cpp?h=5.5#n2389

#ifdef Q_OS_MAC
    case Qt::Key_Enter:
    case Qt::Key_Return:
        // Propagate the enter if you couldn't edit the item and there are no
        // current editors (if there are editors, the event was most likely propagated from it).
        if (!edit(currentIndex(), EditKeyPressed, event) && d->editorIndexHash.isEmpty())
            event->ignore();
        break;
#else
    case Qt::Key_Enter:
    case Qt::Key_Return:
        // ### we can't open the editor on enter, becuse
        // some widgets will forward the enter event back
        // to the viewport, starting an endless loop
        if (state() != EditingState || hasFocus()) {
            if (currentIndex().isValid())
                emit activated(currentIndex());
            event->ignore();
        }
        break;
#endif
@leoetlino

This comment has been minimized.

Show comment
Hide comment
@leoetlino

leoetlino Oct 7, 2017

Member

Changed the fix to just check whether a game is selected or not before emitting the signal.

Member

leoetlino commented Oct 7, 2017

Changed the fix to just check whether a game is selected or not before emitting the signal.

@ligfx

ligfx approved these changes Oct 10, 2017

@leoetlino leoetlino merged commit 53ccd41 into dolphin-emu:master Oct 11, 2017

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details

@leoetlino leoetlino deleted the leoetlino:activate branch Oct 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment