Ticket #43944

qt client on Mac - "Game" menu missing some menu items

Open Date: 2022-02-20 12:51 Last Update: 2022-03-23 20:55

Reporter:
Owner:
Type:
Status:
Closed
Component:
MileStone:
Priority:
5 - Medium
Severity:
5 - Medium
Resolution:
Fixed
File:
5

Details

Cloning this ticket from hostedredmine. Seen in 3.0.0

Game menu of the freeciv-qt client has these 4 items

Save Game alt-S
Save Game As…
Save Map to Image
-----
Leave Game


gtk* clients have 10 items. The biggest missing one is the "Options" item which leads to the "Local Client" and "Remote Server" sub-items, so no way to change Local Client options while playing a game.

Chippo chiphog@gmail.com says it works OK for him "In the QT client on Loonix", so may be Mac only or just some Unices.

Ticket History (3/25 Histories)

2022-02-20 12:51 Updated by: ddeanbrown
  • New Ticket "qt client on Mac - "Game" menu missing some menu items" created
2022-03-01 18:41 Updated by: cazfi
Comment

Without a Mac to reproduce and debug I don't know what I can do to this. Checking the source code for anything obvious, but not much else.

2022-03-01 19:11 Updated by: cazfi
Comment

While cannot debug, can at least make it easier for someone else to do. First such a refactoring patch -> #43996

There's also a small chance that some such refactoring for cleaner code removes actual bug.

That data member named 'menu' (name that is not very namespace-safe) makes me wonder if there could be variable shadowing by some system header in-between. Even --enable-debug does not currently check for shadow warnings on C++ code (as headers of some of the supported Qt versions have those internally)

2022-03-09 23:09 Updated by: cazfi
Comment

Second refactoring patch: #44062

2022-03-10 00:06 Updated by: cazfi
Comment

Reply To (Anonymous)

https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar

So, on Mac, any menu entry named "Options" gets moved to "Application Menu | Preferences" ?

What we do in that case? Accept that the current behavior is correct for Mac? The existence of this ticket indicates that it's not obvious even for Mac users where the options could be found.

2022-03-10 00:12 Updated by: cazfi
Comment

Also, it's "Options" only in English. I guess the behavior will be different when the menu is constructed by translated texts.

2022-03-10 01:55 Updated by: ddeanbrown
Comment

Mac 3.0.0 version of freeciv with qt client does not have a menu item "Preferences" in the application menu (which is called "freeciv"). That menu has an "About freeciv" menu item, and the other items are generic MacOS stuff not related to freeciv. Because there are other Game menu items missing in addition to "Options", I suspect there is something else causing the problem, and possibly the other refactoring patches may fix it.

2022-03-10 02:16 Updated by: cazfi
Comment

Reply To ddeanbrown

Because there are other Game menu items missing in addition to "Options"

What else is it missing? Note that it isn't even supposed to have as many items as, e.g. , gtk-clients. Qt-client is still missing number of features.

2022-03-10 02:58 Updated by: cazfi
Comment

Reply To ddeanbrown

Mac 3.0.0 version of freeciv with qt client does not have a menu item "Preferences" in the application menu (which is called "freeciv").

That documentation speaks of moving "Options" (matched by the string) to applications menu, where it would fire like it was in the original location. Now, freeciv's "Options" is not an item that would fire, but a submenu that one should be able to browse from the main menu. The documentation does not mention how Qt is supposed to handle that corner case - it's quite possible that it doesn't, but has a buggy results like removing the original "Options" but then being unable to add the replacement.

One thing we could test is just renaming the item from "Options" to something that does not trigger that special behavior. Can you test with the attached test patch "SettableThings.patch"? Has textual dependency on #43996 and #44062

2022-03-10 03:04 Updated by: alienvalkyrie
Comment

https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar

There's a line there saying

You can override this behavior by using the QAction::menuRole() property.

which sounds like it should be possible to solve the whole issue that way; avoiding moving the menu entries / submenus at all.

2022-03-10 03:13 Updated by: cazfi
Comment

Reply To alienvalkyrie

https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar

There's a line there saying

You can override this behavior by using the QAction::menuRole() property.

which sounds like it should be possible to solve the whole issue that way; avoiding moving the menu entries / submenus at all.

Except that the menuRole is property of QAction, not QMenu (submenu).

2022-03-10 06:34 Updated by: ddeanbrown
Comment

We have a winner. I tried renaming "Options" and that fixes the problem, even without the other patches. Then I tested using Unicode zero-width space character in the renaming, and that works, so the rename is invisible. Obviously needs a comment. Here's my suggested fix, line 1020 of menu.cpp

  // \u200B is Unicode zero-width space character, prevents qt from trying,
  // and failing, to move a menu named "Options" to the application menu's
  // "preferences" item on MacOS.
  // see https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar
  menu = menu->addMenu(_("\u200BOptions"));

I'm not savvy enough with git to fix this myself, better if someone else handles it. And can then close this ticket.

Screen shot of fix attached.

(Edited, 2022-03-10 06:35 Updated by: ddeanbrown)
2022-03-10 06:52 Updated by: cazfi
  • Owner Update from (None) to cazfi
  • Resolution Update from None to Accepted
  • Milestone Update from (None) to 3.0.1 (closed)
Comment

Evolved the fix a bit - keep the translatable string as is + have this hack on MacOS only. It would be good if you can confirm that this still works.

2022-03-10 06:56 Updated by: cazfi
Comment

Also a version that I plan to still push to S2_6

2022-03-10 07:12 Updated by: ddeanbrown
Comment

Yes that works and is better. Wasn't sure if you could do #ifdef for OS. And good to keep translatable.

In the end, there's still some differences between qt and gtk - gtk has "Clear Chat Log", "Save Chat Log" and "Reload Tileset", which qt lacks & qt has "Shortcuts" and "Load another tileset", which gtk lacks. Not a big deal.

2022-03-11 03:09 Updated by: cazfi
Comment

- S3_1 & S3_0 version that applies without #44062

2022-03-23 20:50 Updated by: cazfi
Comment

Reply To ddeanbrown

In the end, there's still some differences between qt and gtk - gtk has "Clear Chat Log", "Save Chat Log" and "Reload Tileset", which qt lacks & qt has "Shortcuts" and "Load another tileset", which gtk lacks. Not a big deal.

Qt-client does not have those Chat Log related functionalities, and it implements Reload Tileset functionality as a shortcut. "Shortcuts" and "Load another tileset" are more or less broken Qt-client only features, and also in conflict with some new features we'd like to implement to freeciv.

2022-03-23 20:55 Updated by: cazfi
  • Status Update from Open to Closed
  • Resolution Update from Accepted to Fixed

Edit

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Login