Smooth scrolls are disabled if the user prefers-reduced-motion regardless of fingerprint resistance
Categories
(Core :: Layout: Scrolling and Overflow, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox115 | --- | fixed |
People
(Reporter: dlrobertson, Assigned: dlrobertson)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
Summary
We do not check if we should apply fingerprint resistance before disabling smooth scrolls. Currently if the users system settings indicate prefers-reduced-motion, we will not respect smooth scrolls by default and instead perform instant scrolls.
Steps to reproduce
- Set prefers reduced motion or set
general.smoothScroll
tofalse
- Set
privacy.resistFingerprinting
totrue
- Navigate to https://dlrobertson.com/examples/scrollinto-view-scrollend.html
Actual Results:
Both scrolls run in the script are instant
Expected Results:
Regardless of the value of prefers-reduced-motion
or general.smoothScroll
, we should smooth scroll for the second programmatic scroll.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 1•2 years ago
•
|
||
There will likely be two parts to this fix:
- The easier part:
nsLayoutUtils::IsSmoothScrollingEnabled()
should be updated to check Document::ShouldResistFingerprinting on thePresContext()->Document()
- The harder part: APZ does not have a reference to the
Document
, so we'll need to pass through some extra data through to APZ. I think an extra bit added toScrollMetadata
that indicates "Should resist fingerprinting" would suffice. Then all checks forgeneral_smoothScroll
in APZ could be updated to check the value of the preference and the value of this new bit.
Comment 2•2 years ago
|
||
I would propose a more targeted fix for this.
Firstly: it seems odd to me that we would have two different scroll behaviors depending on what page you are on. Consider someone who has enabled PrefersReducedMotion on the OS, which AFAIK is not the default. On about:newtab they would get instant scrolling, and on example.com they'd be smooth scrolling. That's weird. Instead, I think the scroll behavior should be uniform for the entire browser. This means we don't need to check the PresContext's RFP behavior, or plumb anything through in APZ.
Secondly: It's not RFP's job to override every pref a user can set. We do override a few of them, like user agent overrides, but it's infeasible to try to override every single one a user could change that would alter their fingerprint. So we don't need to override general.smoothScroll - we just need to make sure that the OS preference doesn't leak through in it.
Therefore my proposal would be to change this code here to something like this:
if(nsContentUtils::ShouldResistFingerprinting(
RFPTarget::SmoothScroll, "We use the global RFP pref to maintain consistent scroll behavior in the browser.")) {
Preferences::SetBool(
"general.smoothScroll", true, PrefValueKind::Default);
} else {
// We want prefers-reduced-motion to determine the default
// value of the general.smoothScroll pref. If the user
// changed the pref we want to respect the change.
Preferences::SetBool(
"general.smoothScroll",
!LookAndFeel::GetInt(LookAndFeel::IntID::PrefersReducedMotion, 0),
PrefValueKind::Default);
}
Updated•2 years ago
|
Assignee | ||
Comment 3•2 years ago
|
||
Thanks for the info! That makes sense, and should be much more maintainable
Assignee | ||
Comment 4•2 years ago
|
||
When resist fingerprinting is set, ensure that general.smoothScroll has
a resonable default set.
Comment 5•2 years ago
|
||
(In reply to Tom Ritter [:tjr] from comment #2)
Secondly: It's not RFP's job to override every pref a user can set. We do override a few of them, like user agent overrides, but it's infeasible to try to override every single one a user could change that would alter their fingerprint. So we don't need to override general.smoothScroll - we just need to make sure that the OS preference doesn't leak through in it.
+1 on this -- it should be possible for a user who prefers instant scrolling but otherwise wants to reduce the rest of their fingerprint, to have both prefs customized.
Comment 7•2 years ago
|
||
Backed out for causing modules/libpref failures on e.g. test_stickyprefs.js
- backout: https://hg.mozilla.org/integration/autoland/rev/26638884a6f88528e5b021a7584e2b34c7a9359e
- push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=e65029dd25d09d947c2b4455a9d61b26af18e0bb&selectedTaskRun=WL-GcL2WSFiSiO2JBpwlRQ.0
- failure log: https://treeherder.mozilla.org/logviewer?job_id=415666096&repo=autoland&lineNumber=10342
[task 2023-05-13T13:04:14.203Z] 13:04:14 INFO - TEST-PASS | js/xpconnect/tests/unit/test_isModuleLoaded.js | took 855ms
[task 2023-05-13T13:04:14.282Z] 13:04:14 INFO - Cleaning up profile for /builds/worker/workspace/build/tests/xpcshell/tests/js/xpconnect/tests/unit/test_isModuleLoaded.js folder: /data/local/tmp/test_root/xpc/p/aba2bd5c-7788-4db8-a143-129af21b59d6
[task 2023-05-13T13:04:14.899Z] 13:04:14 INFO - TEST-START | modules/libpref/test/unit/test_stickyprefs.js
[task 2023-05-13T13:04:15.189Z] 13:04:15 INFO - remotexpcshelltests.py | modules/libpref/test/unit/test_stickyprefs.js | 3566 | Launched Test App
[task 2023-05-13T13:04:16.799Z] 13:04:16 INFO - remotexpcshelltests.py | modules/libpref/test/unit/test_stickyprefs.js | 3566 | Application ran for: 0:00:01.899795
[task 2023-05-13T13:04:16.869Z] 13:04:16 WARNING - TEST-UNEXPECTED-FAIL | modules/libpref/test/unit/test_stickyprefs.js | xpcshell return code: 0
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - TEST-INFO took 1969ms
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - >>>>>>>
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - running event loop
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - modules/libpref/test/unit/test_stickyprefs.js | Starting notWrittenWhenUnchanged
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - (xpcshell/head.js) | test notWrittenWhenUnchanged pending (2)
[task 2023-05-13T13:04:16.869Z] 13:04:16 INFO - TEST-PASS | modules/libpref/test/unit/test_stickyprefs.js | notWrittenWhenUnchanged - [notWrittenWhenUnchanged : 41] true === true
Assignee | ||
Comment 8•2 years ago
|
||
Having trouble with the bit that changes the smooth scrolling default as soon as privacy.reducedFingerprinting
is set (or unset). I think I'm going to remove that portion from the patch and file a follow-up bug for that component
Comment 10•2 years ago
|
||
bugherder |
Updated•2 years ago
|
I can still reproduce this issue on Firefox 115.0 and latest Nightly on Ubuntu and Windows, but not on macOS. Am I right to assume that bug 1834307 will address the fix on Windows/Ubuntu ?
Assignee | ||
Comment 12•2 years ago
|
||
(In reply to Ardelean Oana, Desktop QA from comment #11)
I can still reproduce this issue on Firefox 115.0 and latest Nightly on Ubuntu and Windows, but not on macOS. Am I right to assume that bug 1834307 will address the fix on Windows/Ubuntu ?
You should need to restart before the change should go into effect. What are the STR you're following and what are you seeing?
Assignee | ||
Updated•2 years ago
|
I followed the STR from Comment 0 and restarted the browser after applying the preferences.
The previous attachment might be obsolete therefore I am uploading another screen capture. Please let me know if there's anything else I can do to help, thank you.
Assignee | ||
Comment 15•2 years ago
|
||
(In reply to Ardelean Oana, Desktop QA from comment #13)
I followed the STR from Comment 0 and restarted the browser after applying the preferences.
Is this done with a fresh profile with no prior preferences set? privacy.resistFingerprinting
really only impacts the default. It shouldn't override a users configuration (see bug 1832598 comment 2).
Description
•