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

Status: RESOLVED FIXED

(Core :: Graphics, task)

Description

[Jean-Yves Avenard [:jya]]

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.

Comment 1

[Jean-Yves Avenard [:jya]]

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

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

Comment 2

[Nicholas Nethercote [inactive]]

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.

Comment 3

[Jean-Yves Avenard [:jya]]

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

Comment 4

[Nicholas Nethercote [inactive]]

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

Comment 5

[Matt Woodrow (:mattwoodrow)]

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.

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1552643 - P1. Re-organise prefs in sections. r=njn

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.

Comment 7

[Jean-Yves Avenard [:jya]]

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

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1552643 - P3. rename medium_high_event_queue.enabled pref. r=froydnj

To make it more inline other preferences naming.

Depends on D31995

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1552643 - P2. Clarify the requirements section. r=njn

Depends on D32251

Comment 10

[Jean-Yves Avenard [:jya]]

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
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

Comment 12

[Raul Gurzau (:RaulG)]

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

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af54b2de7028
Backed out 31 changesets (bug 1552643, bug 1550422) for xpcshell crash on a CLOSED TREE.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e44ab2c46d50
https://hg.mozilla.org/mozilla-central/rev/26e8bca713a3
https://hg.mozilla.org/mozilla-central/rev/70f7e631e8c7