Last Comment Bug 260364 - Fixup radiogroups to use new .value functionality of radio.xml
: Fixup radiogroups to use new .value functionality of radio.xml
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla7
Assigned To: Ian Neal
: John Morrison
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-09-19 08:43 PDT by Ian Neal
Modified: 2011-06-10 08:59 PDT (History)
2 users (show)
iann_bugzilla: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to cookie (1.59 KB, patch)
2004-09-19 08:55 PDT, Ian Neal
iann_bugzilla: review+
iann_bugzilla: superreview+
asa: approval1.8a4+
Details | Diff | Splinter Review
Patch to venkman (1.98 KB, patch)
2004-09-20 06:12 PDT, Ian Neal
rginda: review-
Details | Diff | Splinter Review
Patch to security (7.90 KB, patch)
2004-09-21 13:59 PDT, Ian Neal
kaie: review+
Details | Diff | Splinter Review
Patch to mailnews (18.96 KB, patch)
2004-09-21 17:06 PDT, Ian Neal
neil: review+
Details | Diff | Splinter Review
Patch to mail (5.17 KB, patch)
2004-09-21 17:53 PDT, Ian Neal
mscott: review+
Details | Diff | Splinter Review
Patches to xpfe (2.34 KB, patch)
2004-09-21 17:54 PDT, Ian Neal
iann_bugzilla: review+
iann_bugzilla: superreview+
asa: approval1.8a4+
Details | Diff | Splinter Review
Patch for mailnews v0.2 (15.53 KB, patch)
2004-09-28 13:51 PDT, Ian Neal
iann_bugzilla: review+
mozilla: superreview+
Details | Diff | Splinter Review
Patch to askSendFormat (7.11 KB, patch)
2004-09-28 14:51 PDT, Ian Neal
neil: review-
Details | Diff | Splinter Review
Patch to askSendFormat v0.2 (7.21 KB, patch)
2004-10-13 14:48 PDT, Ian Neal
neil: review+
mozilla: superreview+
Details | Diff | Splinter Review
Unbitrotted pki patch [Checked in: trunk comment 32] (5.09 KB, patch)
2011-06-04 05:54 PDT, Ian Neal
kaie: review+
Details | Diff | Splinter Review

Description Ian Neal 2004-09-19 08:43:26 PDT
Now that the fix to bug 250817 has landed there are certain parts of the code
that could hopefully be simplified now.
Comment 1 Ian Neal 2004-09-19 08:55:49 PDT
Created attachment 159401 [details] [diff] [review]
Patch to cookie

Fixes radiogroup JS in p3p.xul
Comment 2 Ian Neal 2004-09-20 06:12:19 PDT
Created attachment 159479 [details] [diff] [review]
Patch to venkman

Updates venkman-bpprops.js
Comment 3 Ian Neal 2004-09-21 13:59:29 PDT
Created attachment 159625 [details] [diff] [review]
Patch to security

Updates editcerts.js, editemailcert.xul, editsslcert.xul, pref-masterpass.js
and pref-masterpass.xul
Comment 4 Ian Neal 2004-09-21 17:06:22 PDT
Created attachment 159647 [details] [diff] [review]
Patch to mailnews

Patches askSendFormat.js, importDialog.js, am-addressing.js, aw-server.js,
smtpEditOverlay.js, searchTermOverlay.js and am-smime.js
Comment 5 Ian Neal 2004-09-21 17:53:22 PDT
Created attachment 159652 [details] [diff] [review]
Patch to mail

Patches am-smime.js and pref-masterpass.xul in mail tree
Comment 6 Ian Neal 2004-09-21 17:54:36 PDT
Created attachment 159653 [details] [diff] [review]
Patches to xpfe

Patches pref-applications-edit.xul
Comment 7 Ian Neal 2004-09-22 07:01:23 PDT
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe

r+sr=me on attachment 159653 [details] [diff] [review] from Neil via IRC
Comment 8 Ian Neal 2004-09-22 07:49:15 PDT
Comment on attachment 159625 [details] [diff] [review]
Patch to security

for moa
Comment 9 Ian Neal 2004-09-22 08:50:39 PDT
Comment on attachment 159401 [details] [diff] [review]
Patch to cookie

r+sr=neil via IRC
Comment 10 Ian Neal 2004-09-22 08:55:44 PDT
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe

Requesting approval for simple patch
Comment 11 Robert Ginda 2004-09-22 11:13:45 PDT
Comment on attachment 159479 [details] [diff] [review]
Patch to venkman

if this is going to break venkman in older builds, I don't see the point.
Comment 12 Asa Dotzler [:asa] 2004-09-22 11:33:44 PDT
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe

a=asa for checkin to 1.8a4.
Comment 13 Ian Neal 2004-09-22 14:25:44 PDT
Patches to xpfe checked in:
Checking in
mozilla/xpfe/components/prefwindow/resources/content/pref-applications-edit.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-applications-edit.xul,v
 <--  pref-applications-edit.xul
new revision: 1.51; previous revision: 1.50
done
Comment 14 Asa Dotzler [:asa] 2004-09-23 16:17:00 PDT
Comment on attachment 159401 [details] [diff] [review]
Patch to cookie

a=asa for 1.8a4 checkin.
Comment 15 Ian Neal 2004-09-23 17:56:25 PDT
Patch to cookie checked in:
Checking in p3p.xul;
/cvsroot/mozilla/extensions/cookie/resources/content/p3p.xul,v  <--  p3p.xul
new revision: 1.14; previous revision: 1.13
done
Comment 16 neil@parkwaycc.co.uk 2004-09-27 14:30:39 PDT
Comment on attachment 159647 [details] [diff] [review]
Patch to mailnews

>-    else if ( gSearchBooleanRadiogroup.selectedItem.value == 'and')
>+    else if ( gSearchBooleanRadiogroup.value == 'and')
Nit: scope for whitespace cleanup

> function getBooleanAnd()
> {
>-    if (gSearchBooleanRadiogroup.selectedItem)
>-        return (gSearchBooleanRadiogroup.selectedItem.getAttribute("value") == "and") ? true : false;
>+    if (gSearchBooleanRadiogroup.value)
>+        return (gSearchBooleanRadiogroup.value == "and") ? true : false;
> 
>     // default to false
>     return false;
> }
gSearchBooleanRadiogroup.value == "and" already returns true or false; this
entire function call could be inlined into that expression.

askSendFormat needs a separate overhaul; making the radiobutton values
correspond to those in nsIMsgCompose.idl for nsIMsgCompSendFormat would
simplify the code enormously. r=me on everything else with the nits fixed.
Comment 17 Ian Neal 2004-09-28 13:51:20 PDT
Created attachment 160396 [details] [diff] [review]
Patch for mailnews v0.2

Takes on board Neil's comments, patch does not have askSendFormat.js changes
Comment 18 Ian Neal 2004-09-28 13:51:58 PDT
Comment on attachment 160396 [details] [diff] [review]
Patch for mailnews v0.2

Carrying forward r=
Comment 19 Ian Neal 2004-09-28 14:51:18 PDT
Created attachment 160404 [details] [diff] [review]
Patch to askSendFormat

Takes on board Neil's suggestions of overhaul of askSendFormat.js changing the
value for plainTextAndHtml in askSendFormat.xul from 0 to 3 to simplify the
coding.
Comment 20 Ian Neal 2004-09-30 07:46:17 PDT
Checking patch for mailnews v0.2
Checking in base/prefs/resources/content/aw-server.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/aw-server.js,v  <-- 
aw-server.js
new revision: 1.43; previous revision: 1.42
done
Checking in base/prefs/resources/content/am-addressing.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/am-addressing.js,v  <--
am-addressing.js
new revision: 1.8; previous revision: 1.7
done
Checking in base/prefs/resources/content/smtpEditOverlay.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/smtpEditOverlay.js,v  <--
  smtpEditOverlay.js
new revision: 1.27; previous revision: 1.26
done
Checking in base/search/resources/content/searchTermOverlay.js;
/cvsroot/mozilla/mailnews/base/search/resources/content/searchTermOverlay.js,v
<--  searchTermOverlay.js
new revision: 1.37; previous revision: 1.36
done
Checking in extensions/smime/resources/content/am-smime.js;
/cvsroot/mozilla/mailnews/extensions/smime/resources/content/am-smime.js,v  <--
 am-smime.js
new revision: 1.14; previous revision: 1.13
done
Checking in import/resources/content/importDialog.js;
/cvsroot/mozilla/mailnews/import/resources/content/importDialog.js,v  <-- 
importDialog.js
new revision: 1.38; previous revision: 1.37
done
Comment 21 neil@parkwaycc.co.uk 2004-10-13 06:56:31 PDT
Comment on attachment 160404 [details] [diff] [review]
Patch to askSendFormat

This doesn't quite work because param.action is compared identically to an
integer in a switch statement (bypassing automatic conversions). You could
either convert the radiogroup value to an integer before assinging it to
param.action or change MsgComposeCommands to use a type-insensitive comparison.
Comment 22 Ian Neal 2004-10-13 14:48:19 PDT
Created attachment 162007 [details] [diff] [review]
Patch to askSendFormat v0.2

Uses parseInt to make param.action be an integer
Comment 23 neil@parkwaycc.co.uk 2004-10-13 15:44:57 PDT
Comment on attachment 162007 [details] [diff] [review]
Patch to askSendFormat v0.2

I had hoped you'd change the caller ;-)
Comment 24 David :Bienvenu 2004-10-14 07:22:36 PDT
Comment on attachment 162007 [details] [diff] [review]
Patch to askSendFormat v0.2

       if (useDefault)
-	 radioSelect=defaultAction;
+	 format = defaultAction;
       else
-	 radioSelect=param.action;
+	 format = param.action;

now, can't this just be:

var format = (useDefault) ? defaultAction : param.action;

modulo that nit, sr=bienvenu
Comment 25 Ian Neal 2004-10-14 13:33:21 PDT
askSendFormat changes checked in with nit addressed.
Checking in askSendFormat.xul;
/cvsroot/mozilla/mailnews/compose/resources/content/askSendFormat.xul,v  <-- 
askSendFormat.xul
new revision: 1.23; previous revision: 1.22
done
Checking in askSendFormat.js;
/cvsroot/mozilla/mailnews/compose/resources/content/askSendFormat.js,v  <-- 
askSendFormat.js
new revision: 1.21; previous revision: 1.20
done
Comment 26 Ian Neal 2004-10-21 09:44:28 PDT
Comment on attachment 159652 [details] [diff] [review]
Patch to mail

Requesting r= on mail patch
Comment 27 Ian Neal 2004-10-24 10:42:26 PDT
Checking in patch for mail:
Checking in components/prefwindow/content/pref-masterpass.xul;
/cvsroot/mozilla/mail/components/prefwindow/content/pref-masterpass.xul,v  <-- 
pref-masterpass.xul
new revision: 1.4; previous revision: 1.3
done
Checking in extensions/smime/content/am-smime.js;
/cvsroot/mozilla/mail/extensions/smime/content/am-smime.js,v  <--  am-smime.js
new revision: 1.5; previous revision: 1.4
done
Comment 28 Kai Engert (:kaie) 2005-05-22 20:09:58 PDT
Comment on attachment 159625 [details] [diff] [review]
Patch to security

Your change looks correct.

r=kaie assuming you have tested it still works.
Comment 29 Ian Neal 2011-06-04 05:54:11 PDT
Created attachment 537342 [details] [diff] [review]
Unbitrotted pki patch [Checked in: trunk comment 32]

Changes since last version:
* Unbitrotted it.
* Fixed an incorrect = vs == which I spotted during testing elsewhere in the file.
Comment 30 Kai Engert (:kaie) 2011-06-10 05:40:48 PDT
Comment on attachment 537342 [details] [diff] [review]
Unbitrotted pki patch [Checked in: trunk comment 32]

r=kaie

Thanks a lot, especially for finding the =/== issue!
Comment 31 Kai Engert (:kaie) 2011-06-10 05:50:55 PDT
Ian, if for any reason you cannot land the "pki" patch very soon, let's at least land the correctness fix.

If you do land this soon, then please mark bug 663362 fixed, too.
(I've filed that bug primarily for landing it on other branches.)
Thanks!
Comment 32 Ian Neal 2011-06-10 08:58:53 PDT
Comment on attachment 537342 [details] [diff] [review]
Unbitrotted pki patch [Checked in: trunk comment 32]

http://hg.mozilla.org/mozilla-central/rev/b7502e9fbd30

Note You need to log in before you can comment on or make changes to this bug.