50277 - pref-languages should have context enabled/disabled features

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P3)

Description

[Henrik Gemal]

When first entering the pref-panel no languages is selected, which it correct, 
but then the "Remove", "Move Up" and "Move Down" buttons should be disabled.
The same thing if you delete all languages. Then only the "Add" button should be 
enabled.

Comment 1

[Henrik Gemal]

Attachment: Diff file to fix all problem (but only in pref->languages...:)

Comment 2

[Henrik Gemal]

The same is true for other panels (like Smart Browsing, Helper Appl, etc.). 
Should I open seperate bugs or include everything in here?

Comment 3

[sairuh (rarely reading bugmail)]

adding review kw to get on the reviewing radar...

Comment 4

[Henrik Gemal]

please: someone review and checkin, if patch still valid. I've created the
rather big patch more than 3 months ago!

Comment 5

[Blake Ross]

Henrik, there is hardly anyone (if even that) actively querying for the 
patch/review keywords.  You'll need to e-mail the appropriate people for review 
and approval, at which point your patch can be checked in.

Comment 6

[Henrik Gemal]

Attachment: Updated diff to fix context enabled buttons

Comment 7

[Henrik Gemal]

This one shoul almost be ready to be sr and added to moz...

I've fixed all but one js warn:
JavaScript strict warning:
chrome://communicator/content/pref/pref-languages.js line 69: reference to 
undefined property window.arguments

how do I get rid of that one?
The line is:
if (!window.arguments) {

Comment 8

[timeless]

if (!(window && arguments in window && window.arguments)) or something wierd 
like that, there's an example in some of the other recent patches of how to do 
this.

@@ -226,24 +226,25 @@
please reflow the indentation.
+         var arrayOfPrefs = pref_string.split(', ');
is wrong.

+        if (str) {
+          if (!tit) 
+            tit = '[' + str + ']';
+          AddTreeItem(document, active_languages_treeroot, str, tit);
!+             }
!+      }
=
!+        }
!+      }

..
if might be worth caching the document.getElementById("remove"),up,down, 

!   <script language="javascript" 
src="chrome://communicator/content/pref/prefutilities.js"></script>
!   <script language="JavaScript">
=
!   <script type="text/javascript" 
src="chrome://communicator/content/pref/prefutilities.js"/>
!   <script type="text/javascript">
please ^

Comment 9

[Mark Olson]

Henrik, for that last one: |if ("arguments" in window) {| is equivalent

Comment 10

[Mark Olson]

I need a bugzilla to confirm my comments sometimes... My code snippet is
obviously backwards for the situation cited, but you should get the idea.

Comment 11

[Mark Olson]

Attachment: patch - updated against current tree

Comment 12

[Mark Olson]

Ok, the attached patch is simply Henrik Gemal's patch updated against a current
tree.  I noticed that I left out Henrik's name from the contributors lists, so
could whomever checks this in please add it back.

Blake, Ben, this should be ready for r/sr if you would do the honors.

Timeless: I am not convinced caching the getElementById calls would be a very
big win, but I have been wrong before... (often)

Comment 13

[timeless]

try this url:
javascript:for(i=0;i<0;i++)alert(i);alert("".split(',').length)

based on the above, the following is useless :-(
+  if (arrayOfPrefs) {
     for (var i = 0; i < arrayOfPrefs.length; i++) {
+  if (pref_string) {
is probably better. however it would obviously go elsewhere.

please try to wrap lines near 80 cols, even for xul. thanks

Comment 14

[Mark Olson]

Attachment: patch - reformat xul, check for (pref_string) instead of (arrayOfPrefs)

Comment 15

[timeless]

r=timeless
cc alecf for a=

Comment 16

[Blake Ross]

You should be able to use .disabled directly for the buttons, instead of 
set/removeAttribute.  Please do that instead.

Comment 17

[Mark Olson]

Attachment: patch - silence reviewer warnings ;)

Comment 18

[Alec Flett]

sr=alecf

Comment 19

[Blake Ross]

Checked in, with a couple modifications.

Comment 20

[sairuh (rarely reading bugmail)]

vrfy fixed using 2001.02.09.0x comm bits on linux, winnt and mac.