Menu PixelFlasher > Quit PixelFlasher don't work as expected

[knutlegrand]

Menu PixelFlasher > Quit PixelFlasher don't work as expected

[roryokane]

… I would have expected all menus from File to disappear.
Seeing that the exit one is still showing up but not the settings, totally does not make sense.

I don’t know whether it’s macOS or wxPython that added the “Quit PixelFlasher” menu item, but I can guess why either software would do this. I hope my explanation will reduce your confusion.

If a focusable app did not have a “Quit <app>” menu item, the user could only close the app using the Dock or Activity Monitor. This would be analagous to a program on Windows with neither an Exit menu item nor in the top right – the user would be forced to close the program using the Taskbar context menu or Task Manager. It makes sense to prevent this by forcing a Quit menu item to appear.

But why would a Quit menu item be required when windows on macOS generally have a button at their top left corner? It’s because macOS separates the concepts of open windows and open apps. Clicking that red button closes the window but not necessarily the app. For example, Google Chrome can be open without any tabs, and Microsoft Word can be open without any documents. When an app is open with no windows, its menu is still visible at the top of the screen, and apps only enable menu items such as “File” > “Open…” that don’t require an open document.

Why support this complicated concept of apps with no open windows? One benefit is that when a user keeps such an app open, when they later want to open a document in that app, they can avoid having to wait for the app itself to load. (This was a bigger deal when Mac OS was first designed and computers were slower.) Keeping a windowless app open also keeps the app in the Dock as a drag-and-drop target.

Though all apps are quittable, not all apps have preferences (settings). That must be why PixelFlasher’s “Settings” menu item was not forced to appear when it was not registered.

[badabing2005]

Give this one a try.
https://github.com/badabing2005/PixelFlasher/actions/runs/8575437421/artifacts/1389661729

[knutlegrand]

Nothing has changed.
But (both versions) clicking on the red circle (upper left corner of the window) closes the app

[badabing2005]

Fixed in v6.9.1.3

[knutlegrand]

Have installed v6.9.1.3.

• The two first lines of the menu PixelFlasher still refer to files related to Pixel 8 Pro.
• Menu "PixelFlasher > Quit PixelFlasher" still displays a pop-up asking to save last husky OTA file instead of quitting the app.
• Menu "File > Quit" works
• Cmd-Q works

[badabing2005]

Yeah, the "PixelFlasher > Quit PixelFlasher" is the MacOS's not PF's
In fact, with the update, I'm not even telling the OS that the PF's quit is a app quit, somehow it's picking it up.
What MacOS are you on.
This could be a bug in the OS or WxPython, please just use the other options.
Thanks for your feedback.

[roryokane]

With PixelFlasher version 6.9.1.3, I see the exact same bugs as described in the first comment. That is:

• The app menu contains menu items about the Pixel 8 Pro. (I do not have that phone.)
• When I click on the “Quit PixelFlasher” menu item, I am shown a file dialog to save “husky-ota-uq1a.240205.004.a1-75ffdfec.zip”. After pressing Cancel in that dialog, the app still does not quit.

OS: macOS Mojave 10.14.6
Hardware: MacBook Pro (15-inch, 2018)

The release that introduced the bugs

I bisected releases to see when the problem was introduced.

The app menu is buggy in these releases:

• v6.9.1.3
• v6.8.4.0
• v6.7.1.0
• v6.7.0.0

The app menu works as it should in this release:

• v6.6.1.0

The code change that introduced the bugs

The diff between v6.6.1.0 and v6.7.0.0 shows only one commit. The changes in this commit must have caused the bug: 4f6ef29 “- Added history to device image selection (Max 16)”

You may also want to revert whatever menu change you did in 361aae0 “- On MacOS, move the exit menu into PixelFlasher's file menu from the OS's native Exit menu.”. I disagree with the premise of that change – on macOS, the exit menu item should be in the application menu. And that change didn’t work anyway – macOS still placed a “Quit PixelFlasher” menu item in the application menu. In PixelFlasher v6.6.1.0, which doesn’t have that code change, clicking on the “Quit PixelFlasher” menu item in the application successfully quits the app.

[badabing2005]

Thanks for the investigation, but I'm not reverting anything.
There are a lot more menu items now than there was in 6.6.1.0, I'm not removing any features just to cator to MacOS esotheric intricacies.
I added the Exit menu under File menu, just like it is on all other platforms, do not use App menu that MacOS adds, registering the correct menu ID doesn't seem to be working.
I even commented out the registring of the menuid, but someone the OS is still picking it up.
Could be a bug in OS (you're probably using newer OS) or WxPython handling of it.

In any case if this is bothering you, it's open source, feel free to make a PR that properly fixes this without removing any functionality.

[roryokane]

I guess I didn’t mention that the bug I cared about most was that the “About PixelFlasher” menu item was missing. In macOS apps that menu item is always at the top of the application menu, as it was in v6.6.1.0. That was visible in the screenshots, but I didn’t point it out before.

However, I found a workaround for this, too. The About PixelFlasher dialog can be opened from Help > About PixelFlasher:

Now that I know I can get all the same functionality that is normally in the application menu, just in harder-to-remember places, I am less concerned about the application menu containing buggy items.

It would be less confusing for users if the app followed the conventions of each platform it runs on, or if non-supported menus were empty rather than containing buggy menu items. However, I understand that you have limited time to work on PixelFlasher. I hope that at some point, I or another user have time to build the app on macOS and figure out whether a targeted reversion of some of the changes in 4f6ef29 can fix the macOS application menu without breaking any existing functionality.

[badabing2005]

It would be less confusing for users if the app followed the conventions of each platform it runs on, or if non-supported menus were empty rather than containing buggy menu items.

I tried, but if the platform of choice doesn't want to cooperate and insert buggy menu items, what do you do?

Without getting too technical, the way this works in WxPython and MacOS is you create an Exit menu item as wx.ID_EXIT and About menu item as wx.ID_ABOUT
Each of these has unique id, which is different on each platform.
WxPython takes care of registering this ID as Exit or About on MacOS so that they show up under application menu.
The problem lies in the way the id value is translated / handled by the OS or WxPython (totally out of my control), which ends up conflicting with one of the other menu ids.
In this case it happened to conflict with husky download link as stated in this ticket.

I had tried a workaround instead of creating wx.ID_EXIT by creating a unique id using wx.ID_ANY
And then registering this to be an exit menu. wx.App.SetMacExitMenuItemId(exit_item.GetId())
And even though wx.ID_ANY had created absolutely unique ID that did not conflict with any ID, in the translation (wxPython) or the interpretation (MacOS) that ID was translated to match Husky download ID.

I have yet made another workaround, but this will be my last attempt, as I have spent way more time on this than it warrants, people can get used to finding the menu items under File or Help

Please try this build and report back.
https://github.com/badabing2005/PixelFlasher/actions/runs/8690096474

[roryokane]

The About and Quit menu items

The About and Quit menu items in that build work! Thank you.

“About PixelFlasher” successfully opens the dialog, and “Quit PixelFlasher” successfully quits. The keyboard shortcut ⌘Q also still successfully quits.

With this change, “About PixelFlasher” is no longer in the Help menu and “Quit PixelFlasher” is no longer in the File menu:

This makes sense to me: each menu item is only found in one place.

The remaining erroneous menu item

In the application menu, there is still a single erroneous menu item “14.0.0 (UD1A.230803.041, Oct 2023) (Pixel 8 Pro)” that, when clicked, asks where to save “husky-ota-ud1a.230803.041-cca7e111.zip”. (I mention the full name for the aid of future searchers.)

As I think you already knew but I only just understood, this menu item should instead be in the Google Images > Phones > husky (Pixel 8 Pro) > Ota menu, where it is not present. Google’s list of OTA images for husky includes the Oct 2023 one.

At least this is a much less confusing situation than before. Because the application menu has a working “About PixelFlasher” menu item, users will not be trying to open the About PixelFlasher dialog by clicking on random menu items. Some users may may worry that the menu item describes what software will be installed on their phone, or may experimentally click on that menu item and be confused by the download. But if they use the rest of the app and see that it installs the correct software, or if they search for the name of the menu item and find this issue, they should gain confidence that the menu item is harmless.

My understanding of the cause of this bug

Thanks to your explanation of wxPython and IDs, I understand now that the numeric ID generated for this menu item by this line of GoogleImagesMenu.__init__:

PixelFlasher/Main.py

Line 366 in c63f178

menu_id = self.generate_unique_id()

is coinciding with the unique ID of some menu item that belongs in the application menu on macOS according to wxPython.

I also understand that when you previously tried creating a menu item using wx.ID_ANY, which the wxPython documentation said will always be unique, the menu item somehow still ended up being recognized as a special ID whose menu item belongs in the application menu. So I won’t suggest any solutions using wx.ID_ANY.

Possible fixes

Since this menu item is being placed under the first divider of the application menu, I suspect that its ID is conflicting with “Preferences…”, a menu item that macOS apps generally place in that location:

In wxPython’s list of stock items, wx.ID_PREFERENCES looks like the ID for this.

I see two possible ways to prevent this OTA download menu item from being assigned this ID. The fix might only work if both changes are made.

1. Ensure generate_unique_id() cannot return wx.ID_PREFERENCES

Add wx.ID_PREFERENCES to the list of exceptions you added in this commit’s workaround within generate_unique_id():

PixelFlasher/Main.py

Line 262 in c63f178

while unique_id == wx.ID_EXIT or unique_id == wx.ID_ABOUT:

By replacing that line with code like this:

# Workaround for https://github.com/badabing2005/PixelFlasher/issues/187
# This is a subset of https://docs.wxpython.org/stock_items.html#stock-items
WX_PYTHON_STOCK_IDS = [wx.ID_EXIT, wx.ID_ABOUT, wx.ID_PREFERENCES]
while unique_id in WX_PYTHON_STOCK_IDS:

Optional: If we wanted to pre-emptively avoid any other menu item ID collision bugs, we could even add all 72 stock IDs listed in wxPython’s stock items page to WX_PYTHON_STOCK_IDS. The downsides would be that the the list in the code might get out of date as wxPython is updated, so we could not rely on such a fix prevent all such problems in the future, and that we would see irrelevant build errors if wxPython ever removes one of those IDs.

2. Use wx.ID_PREFERENCES for another menu item

When creating PixelFlasher’s “Settings” menu item (the equivalent of “Preferences…”) here:

PixelFlasher/Main.py

Line 1229 in c63f178

config_item = file_menu.Append(wx.ID_ANY, "Settings", "Settings")

Give it wx.ID_PREFERENCES instead of wx.ID_ANY. This will hopefully place this menu item in the application menu on macOS in place of “14.0.0 (UD1A.230803.041, Oct 2023) (Pixel 8 Pro)”.

I don’t have a Python environment set up to test these two possible changes, but let me know if you’d like them in the form of a PR rather than the written description above.

[badabing2005]

Thanks for testing and providing additional details.
I set the settings menu as preferences in this build.
https://github.com/badabing2005/PixelFlasher/actions/runs/8694380600

Give it a try, hopefully this will be the last of this issue :).

[roryokane]

Yep, that seems to have fixed it. Thanks again.

This moved “Settings” menu item still opens the Advanced Configuration Settings dialog, as expected. And “14.0.0 (UD1A.230803.041, Oct 2023) (Pixel 8 Pro)” is back where it belongs within the Google Images menu:

Empty File menu

By the way, this fix has the side effect that there is an empty File menu on macOS:

But I think this situation – a well-organized application menu plus an empty File menu – is better than keeping Settings in the File menu on macOS.

To hide the File menu when it’s not used, it might work to make the self.menuBar.Append(file_menu, "&File") call conditional on file_menu.GetMenuItemCount() or something. But that feels like a hacky workaround. wxPython probably has a simpler way they expect users to add menus that should exist only on certain platforms. I’m not too fussed about the empty menu, so I’m happy to leave exploring those solutions for another time.

[badabing2005]

Happy to hear that the app menu is finally looking right.
As for the file menu being empty.
I'll the proposed method won't work, because as far as wxPython is concerned those items are under File menu, and the count will always be > 0
Instead I'll do the following check

if platform.system() != 'Darwin':

I'm worried that this might cause the items not show under app menu either (not sure).

[roryokane]

I looked for existing solutions to the wxPython empty File menu problem. I didn’t find any, but I did find a lack of a known solution on the wxPython wiki page “Optimizing for Mac OS X”:

That results in […] an "About MyApp" menu item in the Application menu on MacOS. (Note that it also creates an empty "Help" menu, but presumably you can fill that up with, well, Help. :) --MarcHedlund) There are similar tricks for Preferences and Quit menus, which are moved to the "Application" menu.

In this case, we have no other menu items that belong in the File menu to “fill [it] up” with, so that page’s advice doesn’t help. The lack of an answer there suggests that some conditional around self.menuBar.Append(file_menu, "&File") would, sadly, be necessary to hide the empty menu.

[badabing2005]

See if this works.
https://github.com/badabing2005/PixelFlasher/actions/runs/8698861464

[roryokane]

Sadly, it does not. That build successfully hides the File menu on macOS:

But it also hides the Settings menu item from the application menu, making Settings inaccessible from the menu bar:

[badabing2005]

Interesting, and not making sense.
I was afraid that something like this might happen, but I would have expected all menus from File to disappear.
Seeing that the exit one is still showing up but not the settings, totally does not make sense.
But I expect that from Apple ;)