Closed Bug 1730958 Opened 3 years ago Closed 3 years ago

content.sink.pending_event_mode preference looks like it should default 1 on Windows, but it is 0

Categories

(Core :: Preferences: Backend, defect)

defect

Tracking

()

RESOLVED FIXED
94 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox-esr91 94+ fixed
firefox92 --- wontfix
firefox93 --- wontfix
firefox94 --- fixed

People

(Reporter: standard8, Assigned: KrisWright)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

According to the code it looks like the network.notify.IPv6 preference should be 1 on Windows:

https://searchfox.org/mozilla-central/rev/a82dd14b6bc3d05a6a3facc126607e6d34cc2f53/modules/libpref/init/StaticPrefList.yaml#1542-1553

: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:

https://searchfox.org/mozilla-central/rev/a82dd14b6bc3d05a6a3facc126607e6d34cc2f53/modules/libpref/init/StaticPrefList.yaml#124-125

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.

Flags: needinfo?(kwright)
See Also: → 1730960

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.

Flags: needinfo?(kwright)

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.

Assignee: nobody → kwright
Status: NEW → ASSIGNED
Pushed by kwright@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06140961e5db
Fix some #ifdefs in static prefs. r=necko-reviewers,valentin
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch

Is this something we should be backporting to supported branches? Not sure what the implications of these specific prefs being set wrong is.

Flags: needinfo?(valentin.gosu)

(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.

Flags: needinfo?(valentin.gosu)

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.

Flags: needinfo?(kwright)

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:
Flags: needinfo?(kwright)
Attachment #9241444 - Flags: approval-mozilla-esr91?
Attachment #9241444 - Flags: approval-mozilla-beta?

Comment on attachment 9241444 [details]
Bug 1730958 - Fix some #ifdefs in static prefs.

We have no beta left and already built our Release Candidate.

Attachment #9241444 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

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.

Attachment #9241444 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: