956007 - Remove front end for removed x-user-def font preferences

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[[e-mail]]

Bug 213517 removed a bunch of x-user-def font preferences. We need to remove the UI for those preferences.

Comment 1

[Edmund Wong (:ewong)]

Attachment: proposed patch (v1)

Comment 2

[Ian Neal]

Comment on attachment 8357024 [details] [diff] [review]
proposed patch (v1)

What if the user has set the preference to x-user-def, should we be migrating it to another value?

Comment 3

[[e-mail]]

I guess what you could do is that if you find that the pref is not represented in the menulist then you should reset the pref to its default value. (But the pref is only used for the pref UI itself so you don't need to migrate it anywhere else.)

Comment 4

[Edmund Wong (:ewong)]

Do I need to add something more to this patch?

Comment 5

[Ian Neal]

(In reply to Edmund Wong (:ewong) from comment #4)
> Do I need to add something more to this patch?
Yes, as Neil suggested.

Comment 6

[Edmund Wong (:ewong)]

Attachment: proposed patch (v2)

Comment 7

[Edmund Wong (:ewong)]

Attachment: proposed patch(v3)

Comment 8

[[e-mail]]

Comment on attachment 8450016 [details] [diff] [review]
proposed patch(v3)

I can't remember whether the menulist is already initialised when Startup runs. If it is, you can simply check its selectedItem which will be null if the preference value is absent from the list. If it isn't, then you can use getElementsByAttribute to look for a valid value.

I don't know whether you should be using fontLangGroup.valueFromPreferences = undefined; to reset the preference.

Comment 9

[Ian Neal]

Comment on attachment 8450016 [details] [diff] [review]
proposed patch(v3)

With the patch as it is you get the following in the error console:
Error: fontMenuList.getItemFromIndex is not a function
Source File: chrome://communicator/content/pref/pref-fonts.js
Line: 239

You probably meant getItemAtIndex.

When Startup runs, the menulist does not appear to be initialised so when you add a Startup function you would need to use getElementsByAttribute.

As far as I can tell Services.prefs.clearUserPref("font.language.group") and fontLanguage.valueFromPreferences = undefined do exactly the same thing.

Comment 10

[Edmund Wong (:ewong)]

Attachment: proposed patch (v4)

Comment 11

[Ian Neal]

Comment on attachment 8459368 [details] [diff] [review]
proposed patch (v4)

Please use a Startup function instead as suggested by Neil.

Comment 12

[Edmund Wong (:ewong)]

Attachment: proposed patch (v5)

Comment 13

[Ian Neal]

Comment on attachment 8459382 [details] [diff] [review]
proposed patch (v5)

This is just patch 3 again!

Comment 14

[Edmund Wong (:ewong)]

Attachment: proposed patch (v5) for realz!

Comment 15

[Edmund Wong (:ewong)]

Comment on attachment 8459916 [details] [diff] [review]
proposed patch (v5) for realz!

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): None. Removing the
menuitem only. the string can remain in the dtd, as it's removed in the trunk patch.
String changes made by this patch: the string removal will not be included in the aurora push (when approved).

Comment 16

[Ian Neal]

Comment on attachment 8459916 [details] [diff] [review]
proposed patch (v5) for realz!

a=me please make sure you set the correct flags for affected, unaffected, fixed, etc.

Comment 17

[Edmund Wong (:ewong)]

Pushed to comm-aurora:
https://hg.mozilla.org/releases/comm-aurora/rev/1836ecc3e9ee