Skip to content
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

Wrong title shown in window menu #4229

Closed
1 task done
low-batt opened this issue Feb 20, 2023 · 2 comments · Fixed by #4230
Closed
1 task done

Wrong title shown in window menu #4229

low-batt opened this issue Feb 20, 2023 · 2 comments · Fixed by #4230

Comments

@low-batt
Copy link
Contributor

System and IINA version:

  • macOS 13.2.1
  • IINA 1.3.1

Expected behavior:
The window title shown in the window menu as well as the menu displayed when right clicking on the IINA icon in the dock match the title of the window.

Actual behavior:
Some of the time the window titles displayed in both of these menus does not match the title of the window.

Steps to reproduce:

  • Confirm setting Quit after all windows closed is disabled
  • Play a video
  • Close the window
  • Play another video
  • Open the window menu
  • The title displayed belongs to the previous window, not the current video being played
  • MPV does not have this problem.

How often does this happen?
Everytime.

@low-batt low-batt self-assigned this Feb 20, 2023
low-batt added a commit that referenced this issue Feb 20, 2023
This commit will remove the check that suppressed the workaround in
MainWindowController.windowWillOpen for the title problem when running
under macOS 12+.

The workaround was conditionalized because AppKit was confirmed to be
working correctly under macOS 12.3. The assumption was that we could
count of AppKit working correctly going forward. However the problem
is back in macOS 13.2.1. Not clear when it was re-introduced.
@low-batt low-batt linked a pull request Feb 20, 2023 that will close this issue
2 tasks
@low-batt
Copy link
Contributor Author

Analysis

We have been here before. This problem was reported in issues #3159, #3097 and #3253. It was fixed in IINA 1.3.0 by PR #3435.

The window menu is managed by AppKit. The problem occurs when a NSWindow is reused. Apple recommends caching and reusing window objects.

The conclusion was that the root cause was a defect in AppKit. This was backed up by the fact that the window menu was confirmed to be working correctly under macOS Monterey.

PR #3435 added an ugly workaround of setting the window's title to "Window", the default title AppKit assigns to a window when it is first created. As the problem could not be reproduced under Monterey the workaround is only applied to older versions of macOS.

I don't know when the problem reappeared in AppKit, but it is back in macOS Ventura 13.2.1.

Fixing

The fix removes the check in MainWindowController.windowWillOpen that does not apply the workaround when running under macOS 12+. The workaround may not be needed under Monterey, but since the problem is present in Ventura it is possible that updates to Monterey could re-introduce the problem into macOS 12. I am unable to test if the latest version of Monterey already shows this problem.

CarterLi pushed a commit to CarterLi/iina that referenced this issue Mar 1, 2023
This commit will remove the check that suppressed the workaround in
MainWindowController.windowWillOpen for the title problem when running
under macOS 12+.

The workaround was conditionalized because AppKit was confirmed to be
working correctly under macOS 12.3. The assumption was that we could
count of AppKit working correctly going forward. However the problem
is back in macOS 13.2.1. Not clear when it was re-introduced.
MikeWang000000 pushed a commit to MikeWang000000/iina that referenced this issue Mar 8, 2023
This commit will remove the check that suppressed the workaround in
MainWindowController.windowWillOpen for the title problem when running
under macOS 12+.

The workaround was conditionalized because AppKit was confirmed to be
working correctly under macOS 12.3. The assumption was that we could
count of AppKit working correctly going forward. However the problem
is back in macOS 13.2.1. Not clear when it was re-introduced.
svobs pushed a commit to svobs/iina-advance that referenced this issue Mar 13, 2023
This commit will remove the check that suppressed the workaround in
MainWindowController.windowWillOpen for the title problem when running
under macOS 12+.

The workaround was conditionalized because AppKit was confirmed to be
working correctly under macOS 12.3. The assumption was that we could
count of AppKit working correctly going forward. However the problem
is back in macOS 13.2.1. Not clear when it was re-introduced.
low-batt added a commit that referenced this issue Mar 25, 2023
This commit will remove the check that suppressed the workaround in
MainWindowController.windowWillOpen for the title problem when running
under macOS 12+.

The workaround was conditionalized because AppKit was confirmed to be
working correctly under macOS 12.3. The assumption was that we could
count of AppKit working correctly going forward. However the problem
is back in macOS 13.2.1. Not clear when it was re-introduced.

Rebased with develop and corrected merge conflicts.
lhc70000 pushed a commit that referenced this issue Mar 27, 2023
This commit will remove the check that suppressed the workaround in
MainWindowController.windowWillOpen for the title problem when running
under macOS 12+.

The workaround was conditionalized because AppKit was confirmed to be
working correctly under macOS 12.3. The assumption was that we could
count of AppKit working correctly going forward. However the problem
is back in macOS 13.2.1. Not clear when it was re-introduced.

Rebased with develop and corrected merge conflicts.
@uiryuu uiryuu reopened this Mar 27, 2023
@low-batt
Copy link
Contributor Author

IINA 1.3.2 contains the fix for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants