1492284 - Use sharedWorkspace.notificationCenter for Mojave instead of defaultCenter

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, enhancement, P1)

Description

[Hiroyuki Ikezoe (:hiro)]

As per bug 1486971 comment 30, we need to use sharedWorkspace.notificationCenter for Mojave (or later versions) to get the accessibilityDisplayShouldReduceMotion changes instead of defaultCenter.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

A try from bug 1486971 comment 29.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c92696dc85007e12c24c8bee47c94432f0e3fa5f

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1492284 - sharedWorkspace.notificationCenter for Mojave to get the accessibilityDisplayShouldReduceMotion changes. r=mstange

On Mojave (or later versions) defaultCenter doesn't get the
accessibilityDisplayShouldReduceMotion changes, we need to use
sharedWorkspace.notificationCenter instead.

Note that the reason why we use defaultCenter originally is that using
sharedWorkspace.notificationCenter to post a notification (for mochitest)
crashed on MacOSX 10.10 (on a try).

Comment 3

[Haik Aftandilian [:haik]]

Regarding the dependency on bug 1384336 "Avoid running the appshell in content processes", in case it isn't clear, that fix was landed for Mac with bug 1426100 which is in build 63. If this bug blocks 1426100, we'll need it in 63.

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 9010073 [details]
Bug 1492284 - sharedWorkspace.notificationCenter for Mojave to get the accessibilityDisplayShouldReduceMotion changes. r=mstange

Markus Stange [:mstange] has approved the revision.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e570bfcc0ea1
sharedWorkspace.notificationCenter for Mojave to get the accessibilityDisplayShouldReduceMotion changes. r=mstange

Comment 6

[Narcis Beleuzu [:NarcisB]]

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

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 9010073 [details]
Bug 1492284 - sharedWorkspace.notificationCenter for Mojave to get the accessibilityDisplayShouldReduceMotion changes. r=mstange

Approval Request Comment
[Feature/Bug causing the regression]: None
[User impact if declined]: Similar to bug 1486971, but users on MacOSX 10.14 won't see prefers-reduced-motion changes until the browser restarts
[Is this code covered by automated tests?]: Yes, a test in bug 1486971 covers this but we haven't run it on MacOSX 10.14 on our infra
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]:  The change is guarded by OS version checks and a contributor (Sören) has confirmed it works fine on MacOSX 10.14 (in bug 1486971 comment 30)
[String changes made/needed]: None

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 9010073 [details]
Bug 1492284 - sharedWorkspace.notificationCenter for Mojave to get the accessibilityDisplayShouldReduceMotion changes. r=mstange

P1, verified on Nightly + Mojave ships today, uplift approved for 63 beta 9, thanks.

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ff52a0eb02c1