1691171 - "warning: 'beginSheet:modalForWindow:modalDelegate:didEndSelector:contextInfo:' is deprecated" in nsCocoaWindow.mm

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, task, P5)

Description

[Markus Stange [:mstange]]

10:04.52 /Users/mstange/code/mozilla/widget/cocoa/nsCocoaWindow.mm:817:18: warning: 'beginSheet:modalForWindow:modalDelegate:didEndSelector:contextInfo:' is deprecated: first deprecated in macOS 10.10 - Use -[NSWindow beginSheet:completionHandler:] instead [-Wdeprecated-declarations]
10:04.52           [NSApp beginSheet:mWindow
10:04.52                  ^
10:04.52 /Users/mstange/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSApplication.h:584:1: note: 'beginSheet:modalForWindow:modalDelegate:didEndSelector:contextInfo:' has been explicitly marked deprecated here
10:04.52 - (void)beginSheet:(NSWindow *)sheet modalForWindow:(NSWindow *)docWindow modalDelegate:(nullable id)modalDelegate didEndSelector:(nullable SEL)didEndSelector contextInfo:(null_unspecified void *)contextInfo NS_DEPRECATED_MAC(10_0, 10_10, "Use -[NSWindow beginSheet:completionHandler:] instead");
10:04.53 ^

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1691171 - Use -[NSWindow beginSheet:completionHandler:] instead of the deprecated NSApplication method. r=spohl

Depends on D104302

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f55bee1695e6
Use -[NSWindow beginSheet:completionHandler:] instead of the deprecated NSApplication method. r=spohl

Comment 3

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/f55bee1695e6

Comment 4

[Kai Engert [:KaiE:]]

Reopening. Per Magnus Melin, the Thunderbird team would like to ask for a backout of this patch from mozilla-central and mozilla-esr91.

Please see bug 1737489 and bug 1715740 for the justifications.

Comment 5

[Kai Engert [:KaiE:]]

Attachment: Bug 1691171 - Backout revision from Februray 2021

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backing out is a question for Markus as the bugs in question aren't a Firefox issue (and frankly, I'm worried about unintended consequences for Firefox as we enter RC week).

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Also, we generally don't reopen bugs for backouts until the backout has actually landed.

Comment 8

[Wayne Mery (:wsmwk)]

(In reply to Kai Engert (:KaiE:) from comment #4)

Reopening. Per Magnus Melin, the Thunderbird team would like to ask for a backout of this patch from mozilla-central and mozilla-esr91.

Please see bug 1737489 and bug 1715740 for the justifications.

In bug 1737489 Comment 1 I have summarized some of the impact.

Comment 9

[Markus Stange [:mstange]]

I'm not comfortable uplifting this backout to RC at this stage. Firefox has been running with this patch for 8 months and may have introduced implicit dependencies on the new behavior. When I wrote this fix I did not anticipate any change in behavior, but clearly there is a change in behavior, so the risk of depending on the new behavior exists.

If we do land this backout (or if we find a different fix) I would recommend two weeks of Nightly testing before we uplift it, and a bunch of manual testing with various dialog windows. And we'll also need an automated test in Firefox mochitest-chrome that exercises the code path that Thunderbird depends on.

Comment 10

[Kai Engert [:KaiE:]]

try run for backout started:
https://treeherder.mozilla.org/jobs?repo=try&revision=6f2a56506ada6ca98c66aa6b54f685a47b6231d4
(patch should only affect macos, so I canceled other platforms)

Comment 11

[Magnus Melin [:mkmelin]]

Markus, do you have any hints of

• what the root problem might be, what to look at
• if there are existing similar tests somewhere that could be built upon

Comment 12

[Kai Engert [:KaiE:]]

Given comment 9, the earliest we could backout from mozilla-esr91 is in 3 weeks.
I think Thunderbird needs a quicker fix.

Magnus had the idea to change the code to use #ifdef MOZ_THUNDERBIRD (old code) #else (new code) #endif,
this way we could backout for Thunderbird immediately, and avoid immediate changes for Firefox.

I'll submit such a patch to bug 1737489, and maybe we should continue the discussions in bug 1737489.