Skip to content

Commit

Permalink
Fix setting initial window size causes crash at startup, #5250 (#5259)
Browse files Browse the repository at this point in the history
This commit will:
- Add an enableAllMenus method to MenuController
- Add a disableMenus parameter to Utility.showAlert
- Change Utility.showAlert to disable menus while the alert is being
  shown if caller set disableMenus to true
- Change calls to Utility.showAlert in MPVController methods used
  during startup to use the new disableMenus parameter

The MPVController will show alerts when mpv reports errors. This can
happen while PlayerCore.first is being created. Disabling menus prevents
the MenuController from accessing PlayerCore.first while it is in the
process of being created which was the source of the crash reported in
the issue.
  • Loading branch information
low-batt authored Nov 10, 2024
1 parent 0c5825e commit f3cc1c8
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 13 deletions.
9 changes: 5 additions & 4 deletions iina/MPVController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ class MPVController: NSObject {
setOptionString("config", "yes", level: .verbose)
let status = setOptionString(MPVOption.ProgramBehavior.configDir, userConfDir)
if status < 0 {
Utility.showAlert("extra_option.config_folder", arguments: [userConfDir])
Utility.showAlert("extra_option.config_folder", arguments: [userConfDir], disableMenus: true)
}
}

Expand All @@ -543,13 +543,13 @@ class MPVController: NSObject {
let status = setOptionString(op[0], op[1])
if status < 0 {
Utility.showAlert("extra_option.error", arguments:
[op[0], op[1], status])
[op[0], op[1], status], disableMenus: true)
}
}
log("Set user configured mpv option values")
}
} else {
Utility.showAlert("extra_option.cannot_read")
Utility.showAlert("extra_option.cannot_read", disableMenus: true)
}
}

Expand Down Expand Up @@ -1589,7 +1589,8 @@ class MPVController: NSObject {
}

if code < 0 {
Utility.showAlert("mpv_error", arguments: [String(cString: mpv_error_string(code)), "\(code)", name])
Utility.showAlert("mpv_error", arguments: [String(cString: mpv_error_string(code)), "\(code)", name],
disableMenus: true)
}

if sync {
Expand Down
22 changes: 14 additions & 8 deletions iina/MenuController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -968,22 +968,28 @@ class MenuController: NSObject, NSMenuDelegate {

/// Disable all menu items.
///
/// This method is used during application termination to stop any further input from the user.
/// This method is used during application termination to stop any further input from the user and when displaying alerts.
func disableAllMenus() {
isDisabled = true
disableAllMenuItems(NSApp.mainMenu!)
setIsEnabledInAllMenuItems(NSApp.mainMenu!, false)
}

/// Disable all menu items in the given menu and any submenus.
func enableAllMenus() {
isDisabled = false
setIsEnabledInAllMenuItems(NSApp.mainMenu!, true)
}

/// Set `isEnabled` to the given value in all menu items in the given menu and any submenus.
///
/// This method recursively descends through the entire tree of menu items disabling all items.
/// - Parameter menu: Menu to disable
private func disableAllMenuItems(_ menu: NSMenu) {
/// This method recursively descends through the entire tree of menu items setting `isEnabled` in all items.
/// - Parameter menu: Menu to disable or enable.
/// - Parameter value: Value to set `isEnabled` to.
private func setIsEnabledInAllMenuItems(_ menu: NSMenu, _ value: Bool) {
for item in menu.items {
if item.hasSubmenu {
disableAllMenuItems(item.submenu!)
setIsEnabledInAllMenuItems(item.submenu!, value)
}
item.isEnabled = false
item.isEnabled = value
}
}
}
13 changes: 12 additions & 1 deletion iina/Utility.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Utility {
typealias InputValidator<T> = (T) -> ValidationResult

// MARK: - Logs, alerts
static func showAlert(_ key: String, comment: String? = nil, arguments: [CVarArg]? = nil, style: NSAlert.Style = .critical, sheetWindow: NSWindow? = nil, suppressionKey: PK? = nil) {
static func showAlert(_ key: String, comment: String? = nil, arguments: [CVarArg]? = nil, style: NSAlert.Style = .critical, sheetWindow: NSWindow? = nil, suppressionKey: PK? = nil, disableMenus: Bool = false) {
let alert = NSAlert()
if let suppressionKey = suppressionKey {
// This alert includes a suppression button that allows the user to suppress the alert.
Expand Down Expand Up @@ -70,11 +70,22 @@ class Utility {
}

alert.alertStyle = style

// If an alert occurs early during startup when the first player core is being created then
// menus must be disabled while the alert is shown as opening certain menus will cause the menu
// controller to attempt to access the player core while it is being initialized resulting in a
// crash. See issue #5250.
if disableMenus {
AppDelegate.shared.menuController.disableAllMenus()
}
if let sheetWindow = sheetWindow {
alert.beginSheetModal(for: sheetWindow)
} else {
alert.runModal()
}
if disableMenus {
AppDelegate.shared.menuController.enableAllMenus()
}

// If the user asked for this alert to be suppressed set the associated preference.
if let suppressionButton = alert.suppressionButton, suppressionButton.state == .on {
Expand Down

0 comments on commit f3cc1c8

Please sign in to comment.