Closed Bug 956007 Opened 7 years ago Closed 6 years ago
Remove front end for removed x-user-def font preferences
Bug 213517 removed a bunch of x-user-def font preferences. We need to remove the UI for those preferences.
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?
Attachment #8357024 - Flags: review?(iann_bugzilla) → review+
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.)
Do I need to add something more to this patch?
(In reply to Edmund Wong (:ewong) from comment #4) > Do I need to add something more to this patch? Yes, as Neil suggested.
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 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.
Attachment #8450016 - Flags: review?(iann_bugzilla) → review-
Comment on attachment 8459368 [details] [diff] [review] proposed patch (v4) Please use a Startup function instead as suggested by Neil.
Attachment #8459368 - Flags: review?(iann_bugzilla) → review-
Comment on attachment 8459382 [details] [diff] [review] proposed patch (v5) This is just patch 3 again!
Attachment #8459382 - Flags: review?(iann_bugzilla) → review-
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).
Attachment #8459916 - Flags: approval-comm-aurora?
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.
Attachment #8459916 - Flags: approval-comm-aurora? → approval-comm-aurora+
Pushed to comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/1836ecc3e9ee
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.