content.sink.pending_event_mode preference looks like it should default 1 on Windows, but it is 0
Categories
(Core :: Preferences: Backend, defect)
Tracking
()
People
(Reporter: standard8, Assigned: KrisWright)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta-
RyanVM
:
approval-mozilla-esr91+
|
Details | Review |
According to the code it looks like the network.notify.IPv6
preference should be 1 on Windows:
:mozbugz found that this is set to 0 for them on windows.
Looking at the top of the file, I found this bit of documentation:
So, the spaces in between the #
and ifdef
mean that it doesn't apply, and that preference is set to true on all platforms.
Looks like this has been in the code for a couple of years in its current state.
Assignee | ||
Comment 1•3 years ago
|
||
A couple years ago we converted a large amount of prefs to static prefs. I did some of these conversions automatically with a script, so it's possible I overlooked some bad formatting. It looks like affected prefs are here and here. I'm pretty sure this was done by mistake.
Assignee | ||
Comment 2•3 years ago
|
||
It looks like these were formatted wrong by mistake, and so the final else
value in both instances was being used as the default value on all platforms.
Updated•3 years ago
|
Updated•3 years ago
|
Pushed by kwright@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/06140961e5db Fix some #ifdefs in static prefs. r=necko-reviewers,valentin
Comment 5•3 years ago
|
||
bugherder |
Comment 6•3 years ago
|
||
Is this something we should be backporting to supported branches? Not sure what the implications of these specific prefs being set wrong is.
Comment 7•3 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
Is this something we should be backporting to supported branches? Not sure what the implications of these specific prefs being set wrong is.
The ipv6 pref was added in bug 1245059.
It is more of a workaround for an issue I'm not completely sure exists anymore (original bug was reported on windows 7, which is still supported - we haven't investigated if others have the same problem).
Given this was broken for so long means it's probably not causing too many issues, but the safest course of action would be to backport it, at least to ESR.
Comment 8•3 years ago
|
||
Please nominate this for Beta & ESR91 approval when you get a chance. We may want to get a bug on file for investigating whether this pref is even needed anymore, though. Not sure if we can add Telemetry probes which could help inform that decision.
Assignee | ||
Comment 9•3 years ago
|
||
Comment on attachment 9241444 [details]
Bug 1730958 - Fix some #ifdefs in static prefs.
Beta/Release Uplift Approval Request
- User impact if declined: Pref will continue to be incorrectly set
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Flips a pref to its correct state
- String changes made/needed:
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: Pref incorrectly set in ESR
- User impact if declined: Pref will continue to work incorrectly
- Fix Landed on Version: 94
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Simply flips a pref to its correct state. We don't have any mechanism in place to verify how often the pref is even being used.
- String or UUID changes made by this patch:
Comment 10•3 years ago
|
||
Comment on attachment 9241444 [details]
Bug 1730958 - Fix some #ifdefs in static prefs.
We have no beta left and already built our Release Candidate.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 11•3 years ago
|
||
Comment on attachment 9241444 [details]
Bug 1730958 - Fix some #ifdefs in static prefs.
Sets the pref to what it was meant to be, though it's still questionable to me whether we need it given how long it's been incorrectly set. Approved for 91.3esr.
Comment 12•3 years ago
|
||
bugherder uplift |
Updated•3 years ago
|
Description
•