Closed Bug 429667 Opened 17 years ago Closed 17 years ago

make EnableTextbox() available in all prefpanes

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: bugzilla, Assigned: bugzilla)

Details

Attachments

(1 file, 1 obsolete file)

EnableTextbox(aTextboxId, aValue, aFocus) can be used in all prefpanes, where a checkbox is used to enable/disable a textbox, so it's useful to have it available everywhere and don't have redundant code.
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
Adds a new preferences.js and loads its content into preferences.xul.
Assignee: nobody → aqualon
Status: NEW → ASSIGNED
Attachment #316423 - Flags: superreview?(neil)
Attachment #316423 - Flags: review?(iann_bugzilla)
Comment on attachment 316423 [details] [diff] [review] patch v1 Some uncalled for nits: ;-) >Index: suite/common/pref/preferences.js >=================================================================== >+// The contents of this file will be loaded into the scope of the >+// prefwindow and is available in all prefpanes! "content ... be available to ..." But IanN and Neil should know even better. ;-) >+function EnableTextbox(aTextboxId, aValue, aFocus) I'd rename aValue to aCheckboxValue to make its use more clear. >+ let enable = aValue && !pref.locked; I'd use "enabled", because it's a state. >Index: suite/common/pref/preferences.xul >=================================================================== > <stringbundle id="bundle_prefutilities" > src="chrome://communicator/locale/pref/prefutilities.properties"/> > >+ <script type="application/x-javascript" src="chrome://communicator/content/pref/preferences.js"/> >+ The <script> should be the first child.
Comment on attachment 316423 [details] [diff] [review] patch v1 r=me with mnyromyr's nits fixed
Attachment #316423 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 316423 [details] [diff] [review] patch v1 I don't quite agree with Mnyromyr! >+// The contents of this file will be loaded into the scope of the >+// prefwindow and is available in all prefpanes! I'd swap things so you get "is loaded" and "will be available". >+function EnableTextbox(aTextboxId, aValue, aFocus) I'd call it aEnable rather than aCheckbox.
Attachment #316423 - Flags: superreview?(neil) → superreview+
Attached patch patch v2 (deleted) — Splinter Review
Patch, addressing the review comments (I took Neil's suggestions).
Attachment #316423 - Attachment is obsolete: true
Keywords: checkin-needed
Landed v2 on trunk.
Keywords: checkin-needed
Thx for landing -> RESOLVED FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: