Closed Bug 1552643 Opened 3 months ago Closed 3 months ago

old gfxPrefs moved to StaticPrefList.h should be organised by section

Categories

(Core :: Graphics, task)

task
Not set

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Regressed 1 open bug)

Details

Attachments

(3 files)

Following bug 1550422, gfxPrefs have been moved to StaticPrefsList.h

StaticPrefsList is organised through sections, and then by alphabetical order.

gfxPrefs was supposed to be only organised in alphabetical order, however this isn't consistent.

StaticPrefs should be sorted by sections.

:mattwoodrow, :jmuizelaar is this something you could take care of?

I'm not familiar enough with most of those prefs to organised them properly.

Flags: needinfo?(matt.woodrow)
Flags: needinfo?(jmuizelaar)

This comment explains the sectioning:
https://searchfox.org/mozilla-central/source/modules/libpref/init/StaticPrefList.h#15-18

It's not complicated. All the prefs that start with "apz." should be in their own section, those that start with "gfx." in another section, etc. It doesn't require insight into the meaning of individual prefs.

(In reply to Nicholas Nethercote [:njn] from comment #2)

It's not complicated. All the prefs that start with "apz." should be in their own section, those that start with "gfx." in another section, etc. It doesn't require insight into the meaning of individual prefs.

I wish it was that simple. If you look at the current gfxPrefs, you'll see that many don't fall into any classification.

accessibility.browsewithcaret should that be merged with the existing accessibility pref or not.

like the vr pref, should they really be in dom. ?

I don't see much advantage in having dom.vr.enabled alongside dom.animations.*

and slider.snapMultiplier , a section on its own?

It is my believe that many of those prefs ought to be renamed.

All this is well beyond the scope of merging gfxPrefs and StaticPrefs and why I have created this bug.

Summary: StaticPrefList.h should be organised by section → old gfxPrefs moved to StaticPrefList.h should be organised by section

It is that simple. There is a separate section for each unique initial pref segment. If "slider.snapMultiplier" is the only pref with a "slider." initial segment, then yes, it gets its own section.

I agree that some existing prefs have bad names, but as you say that is orthogonal. There is an existing method of organisation for StaticPrefList.h file that is simple and documented. Let's use it :)

Actually renaming prefs is pretty hard, since users with existing set values will go back to the default. That's generally not ideal.

I think we should just do the StaticPrefs sectioning for now.

Flags: needinfo?(matt.woodrow)
Assignee: nobody → jyavenard

This re-organise the various sections, re-sorting preferences by alphabetical order as originally intended.

Note that not all sections were re-organised. Some like the Privacy ones use multiple prefixes. Appropriate teams should revisit those.

Flags: needinfo?(jmuizelaar)

"medium_high_event_queue.enabled", is the only pref using that format. It should be renamed.

Flags: needinfo?(bugs)
See Also: → 1524006
Attachment #9066394 - Attachment description: Bug 1552643 - Re-organise prefs in sections. r?njn! → Bug 1552643 - P1. Re-organise prefs in sections. r?njn!

To make it more inline other preferences naming.

Depends on D31995

renamed it into threads.medium_high_event_queue.enabled as discussed with Nathan over IRC

Flags: needinfo?(bugs)
Attachment #9066916 - Attachment description: Bug 1552643 - P3. Clarify the requirements section. r?njn! → Bug 1552643 - P2. Clarify the requirements section. r?njn!
Attachment #9066915 - Attachment description: Bug 1552643 - P2. rename medium_high_event_queue.enabled pref. r?froydnj → Bug 1552643 - P3. rename medium_high_event_queue.enabled pref. r?froydnj
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aa7086ab09be
P1. Re-organise prefs in sections. r=njn
https://hg.mozilla.org/integration/autoland/rev/caadcd7e02d3
P2. Clarify the requirements section. r=njn
https://hg.mozilla.org/integration/autoland/rev/e30c1aa75529
P3. rename medium_high_event_queue.enabled pref. r=froydnj

This bug was also backed out because backing out only bug 1550422 did not apply cleanly (4 out of 59 hunks FAILED -- saving rejects to file modules/libpref/init/StaticPrefList.h.rej). See https://bugzilla.mozilla.org/show_bug.cgi?id=1550422#c52

Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af54b2de7028
Backed out 31 changesets (bug 1552643, bug 1550422) for xpcshell crash on a CLOSED TREE.
Attachment #9066394 - Attachment description: Bug 1552643 - P1. Re-organise prefs in sections. r?njn! → Bug 1552643 - P1. Re-organise prefs in sections. r=njn
Attachment #9066916 - Attachment description: Bug 1552643 - P2. Clarify the requirements section. r?njn! → Bug 1552643 - P2. Clarify the requirements section. r=njn
Attachment #9066915 - Attachment description: Bug 1552643 - P3. rename medium_high_event_queue.enabled pref. r?froydnj → Bug 1552643 - P3. rename medium_high_event_queue.enabled pref. r=froydnj
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e44ab2c46d50
P1. Re-organise prefs in sections. r=njn
https://hg.mozilla.org/integration/autoland/rev/26e8bca713a3
P2. Clarify the requirements section. r=njn
https://hg.mozilla.org/integration/autoland/rev/70f7e631e8c7
P3. rename medium_high_event_queue.enabled pref. r=froydnj
Regressions: 1567023
You need to log in before you can comment on or make changes to this bug.