Closed Bug 956007 Opened 7 years ago Closed 6 years ago

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

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(seamonkey2.29 wontfix, seamonkey2.30 fixed, seamonkey2.31 fixed)

RESOLVED FIXED
seamonkey2.30
Tracking Status
seamonkey2.29 --- wontfix
seamonkey2.30 --- fixed
seamonkey2.31 --- fixed

People

(Reporter: neil, Assigned: ewong)

Details

Attachments

(1 file, 5 obsolete files)

Bug 213517 removed a bunch of x-user-def font preferences. We need to remove the UI for those preferences.
Status: NEW → ASSIGNED
Attached patch proposed patch (v1) (obsolete) — Splinter Review
Attachment #8357024 - Flags: review?(iann_bugzilla)
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+
Flags: needinfo?(neil)
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.)
Flags: needinfo?(neil)
Do I need to add something more to this patch?
Flags: needinfo?(iann_bugzilla)
(In reply to Edmund Wong (:ewong) from comment #4)
> Do I need to add something more to this patch?
Yes, as Neil suggested.
Flags: needinfo?(iann_bugzilla)
Attached patch proposed patch (v2) (obsolete) — Splinter Review
Attachment #8357024 - Attachment is obsolete: true
Attachment #8449976 - Flags: review?(iann_bugzilla)
Attached patch proposed patch(v3) (obsolete) — Splinter Review
Attachment #8449976 - Attachment is obsolete: true
Attachment #8449976 - Flags: review?(iann_bugzilla)
Attachment #8450016 - Flags: review?(iann_bugzilla)
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-
Attached patch proposed patch (v4) (obsolete) — Splinter Review
Attachment #8450016 - Attachment is obsolete: true
Attachment #8459368 - Flags: review?(iann_bugzilla)
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-
Attached patch proposed patch (v5) (obsolete) — Splinter Review
Attachment #8459368 - Attachment is obsolete: true
Attachment #8459382 - Flags: review?(iann_bugzilla)
Comment on attachment 8459382 [details] [diff] [review]
proposed patch (v5)

This is just patch 3 again!
Attachment #8459382 - Flags: review?(iann_bugzilla) → review-
Attachment #8459382 - Attachment is obsolete: true
Attachment #8459916 - Flags: review?(iann_bugzilla)
Attachment #8459916 - 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+
Target Milestone: --- → seamonkey2.30
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.