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 allow to open the debug window during splashscreen & verification state #8042

Merged
merged 1 commit into from May 23, 2016

Conversation

Projects
None yet
5 participants
@jonasschnelli
Member

jonasschnelli commented May 11, 2016

Fixes #8040.
The debug window can't handle the block verification state during startup (see #8040).

Disabling the debug window until the main application window shows up seems to be a easy and stable solution.

@jonasschnelli jonasschnelli added the GUI label May 11, 2016

@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 11, 2016

Contributor

Do we want to show the menu at all before the app is done initializing?

I think if we swap lines 662 and 663 in bitcoin.cpp the menu won't be created until initialization is done. (But I don't have a Mac to test on.)

app.requestInitialize();
app.createWindow(networkStyle.data());

EDIT: Nvm, it'll take more than that. Asynchronous code is hard.

Contributor

Tyler-Hardin commented May 11, 2016

Do we want to show the menu at all before the app is done initializing?

I think if we swap lines 662 and 663 in bitcoin.cpp the menu won't be created until initialization is done. (But I don't have a Mac to test on.)

app.requestInitialize();
app.createWindow(networkStyle.data());

EDIT: Nvm, it'll take more than that. Asynchronous code is hard.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 11, 2016

Member

@Tyler-Hardin: Right.
And OSX is also picky with the main menu. I'm pretty sure removing the menu will lead into losing the Ctrl-Q / quit option during the startup/splashscreen state.

The menu itself doesn't hurt during the splash screen.
It would just require some work to make the debug window compatible for that state (which is not worth doing that why I propose this PR/simple fix).

Member

jonasschnelli commented May 11, 2016

@Tyler-Hardin: Right.
And OSX is also picky with the main menu. I'm pretty sure removing the menu will lead into losing the Ctrl-Q / quit option during the startup/splashscreen state.

The menu itself doesn't hurt during the splash screen.
It would just require some work to make the debug window compatible for that state (which is not worth doing that why I propose this PR/simple fix).

@fanquake

This comment has been minimized.

Show comment
Hide comment
@fanquake

fanquake May 12, 2016

Member

Tested ACK 29199ad on OS X

However, given that preferences and About bitcoin-qt are clickable, but no window actually shows, do we want to disable those until initialisation is finished as well?

Member

fanquake commented May 12, 2016

Tested ACK 29199ad on OS X

However, given that preferences and About bitcoin-qt are clickable, but no window actually shows, do we want to disable those until initialisation is finished as well?

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 12, 2016

Member

Good point @fanquake!
I have now disabled the about menu as well as the options menu.
Can you re-test?

Member

jonasschnelli commented May 12, 2016

Good point @fanquake!
I have now disabled the about menu as well as the options menu.
Can you re-test?

@fanquake

This comment has been minimized.

Show comment
Hide comment
@fanquake

fanquake May 12, 2016

Member

@jonasschnelli Looks good.
main
debug
Tested ACK 276ce84

Member

fanquake commented May 12, 2016

@jonasschnelli Looks good.
main
debug
Tested ACK 276ce84

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 12, 2016

Member

The menu is supposed to be a property of the main window, it is not supposed to show a menu at all during the splash screen.
Is is possible to fix this more thoroughly instead of disabling individual actions? I can easily see this become a problem again in the future when someone adds an action.

Member

laanwj commented May 12, 2016

The menu is supposed to be a property of the main window, it is not supposed to show a menu at all during the splash screen.
Is is possible to fix this more thoroughly instead of disabling individual actions? I can easily see this become a problem again in the future when someone adds an action.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 12, 2016

Member

On OSX, you will always have a menu bar.

Minimum menu bar would be:
bildschirmfoto 2016-05-12 um 13 26 19

IMO disabling "about" and "debug window" would look better.

Member

jonasschnelli commented May 12, 2016

On OSX, you will always have a menu bar.

Minimum menu bar would be:
bildschirmfoto 2016-05-12 um 13 26 19

IMO disabling "about" and "debug window" would look better.

@MarcoFalke MarcoFalke added the macOS label May 12, 2016

@jonasschnelli jonasschnelli merged commit 276ce84 into bitcoin:master May 23, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

jonasschnelli added a commit that referenced this pull request May 23, 2016

Merge #8042: [Qt] Don't allow to open the debug window during splashs…
…creen & verification state

276ce84 [Qt] Disable some menu items during splashscreen/verification state (Jonas Schnelli)

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Jul 4, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 13, 2016

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