After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 746504 - Critical errors when inserting application error in composer toolbar
Critical errors when inserting application error in composer toolbar
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: composer
master
Other Linux
: Normal normal
: ---
Assigned To: Geary Maintainers
Geary Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-20 05:33 UTC by Robert Schroll
Modified: 2015-03-20 23:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Avoid critical errors when creating multiple application menus (7.12 KB, patch)
2015-03-20 22:25 UTC, Robert Schroll
committed Details | Review

Description Robert Schroll 2015-03-20 05:33:59 UTC
We're getting several critical errors when we create the extra gear menu for the composer toolbar.  I think they're coming because the pill toolbar expects the action to come from the action group is was initialized with.  But for this, we're reaching out to use a GearyController action.
Comment 1 Jim Nelson 2015-03-20 20:20:41 UTC
I don't see this.  How do you reproduce this?  Is functionality missing because of it?
Comment 2 Robert Schroll 2015-03-20 20:32:30 UTC
This only happens when you're running on a system that doesn't provide an app menu, so we create the gear menu.  It occurs whenever a composer is created.

I'm just about to look into this further, so you can not worry about it until you hear from me again.
Comment 3 Robert Schroll 2015-03-20 20:49:44 UTC
There are two critical messages and one warning message all related to trying to set the tooltip of the gear menu.  This is because it's looking in the composer's actions for the gear menu action, when it's in the controller.  This doesn't impact usability at all, though, because there's no tooltip set for this button anyway.  If we want to get rid of these, we could put a test for null before we do anything with the tooltips.

There is one more warning message related to the fact that we've already attached the menu to the original gear button.  This doesn't seem to cause any problems right now, but I think it's a bit more worrisome.  Two ideas off the top of my header -- we could make a new menu for each gear menu, or we could figure out how to remove the menu from the existing button before adding it to a new one (and then put it back afterwards).
Comment 4 Robert Schroll 2015-03-20 22:25:53 UTC
Created attachment 300004 [details] [review]
Avoid critical errors when creating multiple application menus

This gets rid of all the warnings.  As a bonus, it reduces the 
duplication between toolbar_menu.ui and app_menu.interface.
Comment 5 Jim Nelson 2015-03-20 22:42:54 UTC
Review of attachment 300004 [details] [review]:

Nice!
Comment 6 Robert Schroll 2015-03-20 22:50:56 UTC
Attachment 300004 [details] pushed as 98be06d - Avoid critical errors when creating multiple application menus
Comment 7 Vadim Rutkovsky 2015-03-20 23:12:18 UTC
This commit breaks geary build:

-- Installing: /ostbuild/results/usr/share/geary/ui/toolbar_mark_menu.ui
CMake Error at ui/cmake_install.cmake:104 (FILE):
  file INSTALL cannot find "/ostbuild/source/geary/ui/toolbar_menu.ui".
Call Stack (most recent call first):
  cmake_install.cmake:44 (INCLUDE)

See http://build.gnome.org/continuous/buildmaster/builds/2015/03/20/65/build/log-geary.txt
Comment 8 Vadim Rutkovsky 2015-03-20 23:14:42 UTC
Please revert https://git.gnome.org/browse/gnome-continuous/commit/?id=19b196713c5139f6ebfe4aed5efff116c49e0404 after the build is fixed
Comment 9 Robert Schroll 2015-03-20 23:22:11 UTC
I've gone ahead and pushed this directly.
Comment 10 Robert Schroll 2015-03-20 23:29:17 UTC
Sorry, "this" meant bbfa2c63.  I thought git-bz would put that in, but it only includes the comment if the commit was previously attached as a patch.

I think I've done the right thing in reverting gnome-continuous.  If not, please let me know what I screwed up.
Comment 11 Vadim Rutkovsky 2015-03-20 23:39:21 UTC
(In reply to Robert Schroll from comment #10)
> Sorry, "this" meant bbfa2c63.  I thought git-bz would put that in, but it
> only includes the comment if the commit was previously attached as a patch.
> 
> I think I've done the right thing in reverting gnome-continuous.  If not,
> please let me know what I screwed up.

All good, it was built in http://build.gnome.org/#/build/20150320.68.

Thanks for a fast response!