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

Only update dbus menu when it has been changed #10070

Merged
merged 2 commits into from Jul 31, 2017

Conversation

Projects
None yet
6 participants
@zcbenz
Contributor

zcbenz commented Jul 20, 2017

The dbus menu implementaion of KDE would send the about-to-show signal whenever the menu has been changed, resulting in infinite loop on our side.

Close #8455.

zcbenz added some commits Jul 20, 2017

Only update dbus menu when it has been changed
The dbus menu implementaion of KDE would send the about-to-show signal
whenever the menu has been changed, resulting in infinite loop on our
side.
@ChALkeR

This comment has been minimized.

Show comment
Hide comment
@ChALkeR

ChALkeR Jul 22, 2017

@zcbenz Thanks for this!
FWIW, I will try to test this in a few days… I can't promise that I would be able to, unfortunately.

Also perhaps /cc @kbroulik.

ChALkeR commented Jul 22, 2017

@zcbenz Thanks for this!
FWIW, I will try to test this in a few days… I can't promise that I would be able to, unfortunately.

Also perhaps /cc @kbroulik.

@SeptemberHX

This comment has been minimized.

Show comment
Hide comment
@SeptemberHX

SeptemberHX Jul 25, 2017

Hope this works !!
can't wait for the global menu of electron app.

SeptemberHX commented Jul 25, 2017

Hope this works !!
can't wait for the global menu of electron app.

@zcbenz zcbenz merged commit a7035b0 into master Jul 31, 2017

7 of 9 checks passed

electron-win-x64 Build #3601 failed in 15 min
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
electron-linux-arm Build #7356431 succeeded in 82s
Details
electron-linux-ia32 Build #7356432 succeeded in 79s
Details
electron-linux-x64 Build #7356433 succeeded in 201s
Details
electron-mas-x64 Build #4621 succeeded in 12 min
Details
electron-osx-x64 Build #4623 succeeded in 10 min
Details
electron-win-ia32 Build #3611 succeeded in 12 min
Details

@zcbenz zcbenz deleted the fix-dbus-menu branch Jul 31, 2017

@gasinvein

This comment has been minimized.

Show comment
Hide comment
@gasinvein

gasinvein Jul 31, 2017

Will this be included in next release?

gasinvein commented Jul 31, 2017

Will this be included in next release?

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Jul 31, 2017

Contributor

Will this be included in next release?

Yes.

@zeke @jkleinsc I think we should also backport this to v1.6.x since the problem was quite annoying.

Contributor

zcbenz commented Jul 31, 2017

Will this be included in next release?

Yes.

@zeke @jkleinsc I think we should also backport this to v1.6.x since the problem was quite annoying.

@michaldybczak

This comment has been minimized.

Show comment
Hide comment
@michaldybczak

michaldybczak Jul 31, 2017

We've been waiting for this since many months. I'm happy that finnally it will be fixed, because it affects all electron apps.

michaldybczak commented Jul 31, 2017

We've been waiting for this since many months. I'm happy that finnally it will be fixed, because it affects all electron apps.

@nsklaus

This comment has been minimized.

Show comment
Hide comment
@nsklaus

nsklaus Oct 10, 2017

why is this issue closed? this is still not resolved.
both atom stable and beta aren't working correctly under kde (menu frozen, and high cpu usage).

nsklaus commented Oct 10, 2017

why is this issue closed? this is still not resolved.
both atom stable and beta aren't working correctly under kde (menu frozen, and high cpu usage).

@michaldybczak

This comment has been minimized.

Show comment
Hide comment
@michaldybczak

michaldybczak Oct 12, 2017

I suspect it's because version that contains fix finally became a release candidate. Before it was beta so not available in any repo. I expect to be available in few weeks on rolling release distros, hopefully.

michaldybczak commented Oct 12, 2017

I suspect it's because version that contains fix finally became a release candidate. Before it was beta so not available in any repo. I expect to be available in few weeks on rolling release distros, hopefully.

@nsklaus

This comment has been minimized.

Show comment
Hide comment
@nsklaus

nsklaus Oct 14, 2017

@michaldybczak
thanks for the reply. i've updated to kubuntu 17.10 beta, and i confirm it's now working properly. i do get global menu working with atom and 100% cpu problem is gone as well.

nsklaus commented Oct 14, 2017

@michaldybczak
thanks for the reply. i've updated to kubuntu 17.10 beta, and i confirm it's now working properly. i do get global menu working with atom and 100% cpu problem is gone as well.

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