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 705898 - Fix the app menu button
Fix the app menu button
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-08-13 12:09 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-08-13 15:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
panel: Rewrite the app menu to use the new "sync" pattern (9.35 KB, patch)
2013-08-13 12:09 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
panelMenu: Use the accessible-name property instead of a label actor (2.64 KB, patch)
2013-08-13 12:09 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
panelMenu: Use the accessible-name property instead of a label actor (2.45 KB, patch)
2013-08-13 13:23 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
panel: Rewrite the app menu to use the new "sync" pattern (9.22 KB, patch)
2013-08-13 13:23 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-08-13 12:09:25 UTC
The code is just broken. See patches.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-08-13 12:09:27 UTC
Created attachment 251463 [details] [review]
panel: Rewrite the app menu to use the new "sync" pattern

The existing app menu was a kludge of legacy code that tried to manage
a bunch of state, and had a number of issues:

 * It didn't properly manage visibility when combined with multiple
   apps and the overview.

 * It didn't properly manage reactivity when tabbing away from a busy
   app to another app.

 * It didn't properly disconnect signals when going from one app
   to nothing.

and countless others. Rewrite it to use the new "sync" code pattern,
where we centralize all state management and do transitions from that,
rather than strange and quirky control flow.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-08-13 12:09:33 UTC
Created attachment 251464 [details] [review]
panelMenu: Use the accessible-name property instead of a label actor

We already have code for this in StWidget.
Comment 3 Giovanni Campagna 2013-08-13 13:11:23 UTC
Review of attachment 251463 [details] [review]:

Does this work with keynav to the app menu (where the focus is on the stage but we keep the previous app menu visible?)
It doesn't seem to me...
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-08-13 13:23:50 UTC
Created attachment 251487 [details] [review]
panelMenu: Use the accessible-name property instead of a label actor

We already have code for this in StWidget.
Comment 5 Jasper St. Pierre (not reading bugmail) 2013-08-13 13:23:53 UTC
Created attachment 251488 [details] [review]
panel: Rewrite the app menu to use the new "sync" pattern

The existing app menu was a kludge of legacy code that tried to manage
a bunch of state, and had a number of issues:

 * It didn't properly manage visibility when combined with multiple
   apps and the overview.

 * It didn't properly manage reactivity when tabbing away from a busy
   app to another app.

 * It didn't properly disconnect signals when going from one app
   to nothing.

and countless others. Rewrite it to use the new "sync" code pattern,
where we centralize all state management and do transitions from that,
rather than strange and quirky control flow.
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-08-13 13:24:51 UTC
No, Ctrl+Alt+Tab works for me here, but I did find a bug where moving
a window to a different workspace with Ctrl+Alt+Shift+Down made the app
menu disappear. Can you try with this patch set? Order has been reversed
intentionally.
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-08-13 15:22:32 UTC
Attachment 251487 [details] pushed as 33e51cc - panelMenu: Use the accessible-name property instead of a label actor
Attachment 251488 [details] pushed as 7563e04 - panel: Rewrite the app menu to use the new "sync" pattern


Reviewed on IRC.