As Jonathan mentioned in bug 1475462 comment 4, it seems that reloading content doesn't refresh prefers-reduced-motion media query. I thought the patch in bug 1475462 comment 6 fixes the issue, but it doesn't actually (thanks arai for checking it). There must be something different from other platforms.

FWIW, this is the patch I mentioned in comment 0.

I will check this once I get a Macbook Brian will send to me.

This is an odd bug. It seems that the property value, accessibilityDisplayShouldReduceMotion, has never changed since each process is created. Other similar properties, accessibilityDisplayShouldReduceTransparency, accessibilityDisplayShouldDifferentiateWithoutColor, accessibilityDisplayShouldIncreaseContrast are not changed either. Whereas, accessibilityDisplayShouldInvertColors and voiceOverEnabled are changed properly. Another odd thing is that NSSystemColorsDidChangeNotification [1] receives NSWorkspaceAccessibilityDisplayOptionsDidChangeNotification too. I have no idea how Safari handles these oddnesses, I can't see any special handlings for accessibilityDisplayShouldReduceMotion in WebKit. We are doing something special when we create a process on MacOSX? [1] https://searchfox.org/mozilla-central/rev/43969de34f5d4b113133d090f024e5eed7a82af0/widget/cocoa/nsChildView.mm#3335

This is somewhat related to how we create processes on MacOSX. The problem doesn't happen if there is no other firefox instances (releases, nightly, local builds, etc.). For example, if there is a firefox 61 instance and a local build instance with non-E10S mode, the problem happens, but once the firefox 61 instance finished, the problem disappears. This look pretty odd, how the firefox 61 instance affected local build? CCing, Markus and spohl, they might know something.

The property value, accessibilityDisplayShouldReduceMotion is properly updated in the parent processes, but not in child processes. I can see the same symptoms for isSwipeTrackingFromScrollEventsEnabled as well, but it doesn't matter since isSwipeTrackingFromScrollEventsEnabled is only used in the parent process (in browser.xul).

(In reply to Hiroyuki Ikezoe (:hiro) (PTO on Aug 31) from comment #3) > Whereas, > accessibilityDisplayShouldInvertColors and voiceOverEnabled are changed > properly. I was wrong. accessibilityDisplayShouldInvertColors isn't changed at all. So voiceOverEnabled is the only one whose value is correctly changed in the child process. And I did create a simple console application which queries accessibilityDisplayShouldReduceMotion, the value isn't changed in this simple application. So my best guess is that those values are not changed in console applications, but changed in GUI applications. (which means our parent processes are GUI application, whereas child processes are console applications?).

I found setting dom.ipc.useNativeEventProcessing.content false solves this issue.

(In reply to Hiroyuki Ikezoe (:hiro) (PTO on Aug 31) from comment #7) > I found setting dom.ipc.useNativeEventProcessing.content false solves this > issue. Setting the pref *true*.

I could write a fix for this.

I did successfully write mochitest for this. The mochitest needs the fix for bug 1478212. Let's see it on try; https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bda4f252e5f797f68ea2a10911d876902576302

Hmm, the test works fine on MacOS10.13.6, but crashed on the try server because the server is MacOS10.10. Using sharedWorkspace.notificationCenter to post the notification seems to be a problem, so I did use NSNotificationCenter.defaultCenter instead. It works fine! https://treeherder.mozilla.org/#/jobs?repo=try&revision=407f77f0062fb3cc1d21e7cbf75c3f1b4485e737

In child processes on MacOSX we don't spin native event loop at all. Without native event loops NSWorkspace.accessibilityDisplayShouldReduceMotion doesn't return up-to-date value when the system setting changed for some reasons. To workaround this we use NSWorkspace.accessibilityDisplayShouldReduceMotion only on the parent process which spins native event loop or when it's the initial query on the child process. And we give the up-to-date value to the child process via an IPC call just like other cached values do. Depends on D5002

The framework to simulate the setting change works as following; - nsIDOMWindowUtils.setPrefersReducedMotion() calls an IPC function which ends up calling nsChildView::SetPrefersReducedMotion() in the parent process - nsChildView::SetPrefersReducedMotion() sets the given value into nsLookAndFeel::mPrefersReducedMotionCached just like we set the value queried via NSWorkspace.accessibilityDisplayShouldReduceMotion in the parent process and send a notification which is the same notification MacOSX sends when the system setting changed - Normally the cached value is cleared before quering new values since the cache value is stale, but in this case the value is up-to-date one, so nsChildView::SetPrefersReducedMotion() tells that we don't need to clear the cache, and nsIDOMWindowUtils.resetPrefersReducedMotion() resets that state of 'we don't need to clear the cache' There are two test cases with the framework in this commit, one is just setting the value and checking the value queried by window.matchMedia. The other one is receiving 'change' event and checking the value of the event target. Note that to make this test works the patch for bug 1478212 is necessary since the test runs in an iframe. Depends on D5003

Thanks for taking this on. Sorry I didn't notice when I landed the native event loop patch.

Alex, though I actually add the bug into blocker list, as for this 'prefers-reduced-motion', the feature hasn't landed at that moment, so that's not your fault at all. And I can imagine it's really hard to notice it, to me it's impossible, how can we know the fact that some of the system properties can not be delivered without spinning native event loops in a proprietary software without any documents for the fact? Upgrading priority since I'd want to uplift this because users who try 'prefers-reduced-motion' will complain that the feature is totally broken. Normally the system setting is rarely changed, but the users who want to try this new feature will change the setting many times.

Comment on attachment 9006437 [details] Bug 1486971 - Query NSWorkspace.accessibilityDisplayShouldReduceMotion only if it's on parent processes or it's the initial query on child processes. r=mstange Markus Stange [:mstange] has approved the revision.

Comment on attachment 9006435 [details] Bug 1486971 - Receive accessibilityDisplayShouldReduceMotion change and notify it. r=mstange Markus Stange [:mstange] has approved the revision.

If the system wants to update these values, it needs to run code to do that updating. It expects to be able to run this code on the main thread. But we never give it a chance to run the code because we now completely control everything that runs at all times - we never give the system an opportunity to do on demand work. I suppose it could run its updating code synchronously whenever we query the values. But it's not designed for that, it assumes that there is a native event loop.

Comment on attachment 9006438 [details] Bug 1486971 - Test for dynamically change of the prefers-reduced-motion setting on MacOSX. r=mstange Markus Stange [:mstange] has approved the revision.

Comment on attachment 9006438 [details] Bug 1486971 - Test for dynamically change of the prefers-reduced-motion setting on MacOSX. r=mstange Nathan, can you please check the changes in sync-messages.ini? There are new two functions only for test. (I couldn't find a way to add a review request in the revision on Phabricator)

Comment on attachment 9006438 [details] Bug 1486971 - Test for dynamically change of the prefers-reduced-motion setting on MacOSX. r=mstange Nathan Froyd [:froydnj] has approved the revision.

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e92f3e990b35 Receive accessibilityDisplayShouldReduceMotion change and notify it. r=mstange https://hg.mozilla.org/integration/autoland/rev/cea8a92452d5 Query NSWorkspace.accessibilityDisplayShouldReduceMotion only if it's on parent processes or it's the initial query on child processes. r=mstange https://hg.mozilla.org/integration/autoland/rev/67a5acf7363d Test for dynamically change of the prefers-reduced-motion setting on MacOSX. r=froydnj,mstange

https://hg.mozilla.org/mozilla-central/rev/e92f3e990b35 https://hg.mozilla.org/mozilla-central/rev/cea8a92452d5 https://hg.mozilla.org/mozilla-central/rev/67a5acf7363d

Tested with today's Nightly, about:buildconfig says "Built from https://hg.mozilla.org/mozilla-central/rev/73a2f427e2fdd0511198a2e308c21f3b74ca7f7d", so these patches should be included. I also tested with a current copy of mozilla-central and a local build. Firefox still requires a restart to detect a change of the option on macOS Mojave.

(In reply to Sören Hentzschel from comment #26) > Tested with today's Nightly, about:buildconfig says "Built from > https://hg.mozilla.org/mozilla-central/rev/ > 73a2f427e2fdd0511198a2e308c21f3b74ca7f7d", so these patches should be > included. I also tested with a current copy of mozilla-central and a local > build. Firefox still requires a restart to detect a change of the option on > macOS Mojave. Hmm it works fine on MacOSX 10.13.6. Something has changed in Majave? Or, I haven't checked the behavior on chrome content, so there may be other issues (only on MacOSX?). Can you please upload your current patches in bug 1478597?

I tested it independent from bug 1478597 for website content with the following code: ``` <html> <head> </head> <body> <script> const reducedMotion = window.matchMedia("(prefers-reduced-motion)").matches; console.log(reducedMotion); window.matchMedia('(prefers-reduced-motion)').addEventListener('change', event => { console.log('A'); }); window.matchMedia('(prefers-reduced-motion: reduce)').addEventListener('change', event => { console.log('B'); }); window.matchMedia('(prefers-reduced-motion: no-preference)').addEventListener('change', event => { console.log('C'); }); </script> </body> </html> ``` 'A', 'B' and 'C' were never logged. After changing the OS setting and reloading the file the content of the first console.log didn't change. The value only changed after a restart of Firefox.

Sören, can you please try this binary to see if it works or not? https://treeherder.mozilla.org/#/jobs?repo=try&revision=c92696dc85007e12c24c8bee47c94432f0e3fa5f It might be possible that on Mojave or laters `defaultCenter` doesn't receive NSWorkspaceAccessibilityDisplayOptionsDidChangeNotification. We might need to use sharedWorkspace.notificationCenter instead there. Just a guess though.

Good news: with the build from comment #29 it works on macOS Mojave!

(In reply to Sören Hentzschel (not available from 2018/09/20 until 2018/09/27) from comment #30) > Good news: with the build from comment #29 it works on macOS Mojave! Thanks! Filed bug 1492284.

Comment on attachment 9006435 [details] Bug 1486971 - Receive accessibilityDisplayShouldReduceMotion change and notify it. r=mstange Approval Request Comment [Feature/Bug causing the regression]: Bug 1475462 [User impact if declined]: User will not see prefers-reduced-motion changes until the browser restarts [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: I don't think so [List of other uplifts needed for the feature/fix]: All patches in this bug, bug 1478212 and bug 1491580 [Is the change risky?]: Now it's low, actually this patch caused a crash on MacOSX 10.9, but it has already been fixed by bug 1491580. [Why is the change risky/not risky?]: Basically the patch does just observe an notification event, unfortunately it caused the crash on the older version of MacOSX, but it's been already fixed. [String changes made/needed]: None

Note that, FWIW, :Anca has been working on prefers-reduced-motion QA.

Hiro, I see that this uplift request needs also uplifting patches from 2 other bugs, one of them being a P3 and the other a crash caused by the patch you want to uplift. This is not noted as a regression and according to https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion only Safari supports that feature today. Given that it already caused crashes, how important is it to have it in 63 for OSX and not let it ride the trains with 64?

I think _conscious_ web developers on MacOSX try to use the new released prefers-reduced-motion on Firefox 63, and they will think it's completely broken because the feature values don't reflect when the corresponding system setting changes, and they will think Firefox isn't a polished product. Yes, I understand if it caused other crashes the impression will be more worse, but I believe these patches won't cause any other crashes.

I agree with Hiro.

Comment on attachment 9006435 [details] Bug 1486971 - Receive accessibilityDisplayShouldReduceMotion change and notify it. r=mstange Uplift approved for 63 beta 8 based on the assessment in comments #35 and #36, Thanks.

Hiro, please request uplist for the patches in the 2 other bugs (bug 1478212 and bug 1491580), thanks.

Done.

https://hg.mozilla.org/releases/mozilla-beta/rev/8d26699d84cf https://hg.mozilla.org/releases/mozilla-beta/rev/78dc157290d3 https://hg.mozilla.org/releases/mozilla-beta/rev/b84a0e018ea8

I reproduced this issue on Beta 63.0b5 (20180910132416) under macOS 10.13. This is fixed on Beta 63.0b11 (20181001131022) and Nightly 64.0a1 (2018-10-03), under macOS 10.13 and macOS 10.12.