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
Menu PixelFlasher > Quit PixelFlasher don't work as expected #187
Comments
- #187 register exit menu id for MacOS to OS's "Exit" menu. - KernelSU bug fix and improvements. - Improved logging to further easy troubleshooting. - Update banned kernels list. - Update and re-add `Magisk zygote64_32 canary` with a forked [URL](https://raw.githubusercontent.com/ActiveIce/Magisk_zygote64_32/master/canary.json). - Minor typo corrections and other improvements.
Nothing has changed. |
Fixed in v6.9.1.3 |
Have installed v6.9.1.3.
|
Yeah, the "PixelFlasher > Quit PixelFlasher" is the MacOS's not PF's |
With PixelFlasher version 6.9.1.3, I see the exact same bugs as described in the first comment. That is:
OS: macOS Mojave 10.14.6 The release that introduced the bugsI bisected releases to see when the problem was introduced. The app menu is buggy in these releases: The app menu works as it should in this release: The code change that introduced the bugsThe 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: You may also want to revert whatever menu change you did in |
Thanks for the investigation, but I'm not reverting anything. 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. |
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. |
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 I had tried a workaround instead of creating 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. |
The About and Quit menu itemsThe 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 itemIn 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 bugThanks to your explanation of wxPython and IDs, I understand now that the numeric ID generated for this menu item by this line of Line 366 in c63f178
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 Possible fixesSince 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, 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
|
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:
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.
Thanks for testing and providing additional details. Give it a try, hopefully this will be the last of this issue :). |
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 menuBy 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 |
Happy to hear that the app menu is finally looking right.
I'm worried that this might cause the items not show under app menu either (not sure). |
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”:
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 |
See if this works. |
Interesting, and not making sense. |
- Logging improvements - When Magisk delta is installed, automatically set the package to: `io.github.huskydg.magisk`
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. |
When trying to quit PixelFlasher using the menu PixelFlasher > Quit PixelFlasher, a pop-up asking to save last husky OTA file is displayed instead of quitting the app.
The connected device is a Lynx (Pixel 7A) (information which would normally not be related to quitting the app)
The two first lines of the menu PixelFlasher refer to files related to Pixel 8 Pro. I don't know where it is coming from.
Here are 2 screenshots :
MacBook Pro (Retina, 13-inch, Early 2015)
MacOS 12.7.4 (Monterey)
PixelFlasher 6.9.0.2
The text was updated successfully, but these errors were encountered: