The default bug view has changed. See this FAQ.

Fixup radiogroups to use new .value functionality of radio.xml

RESOLVED FIXED in mozilla7

Status

()

Core
XUL
--
minor
RESOLVED FIXED
13 years ago
6 years ago

People

(Reporter: Ian Neal, Assigned: Ian Neal)

Tracking

Trunk
mozilla7
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 3 obsolete attachments)

(Assignee)

Description

13 years ago
Now that the fix to bug 250817 has landed there are certain parts of the code
that could hopefully be simplified now.
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

13 years ago
Assignee: jag → bugzilla
Status: ASSIGNED → NEW
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

13 years ago
Created attachment 159401 [details] [diff] [review]
Patch to cookie

Fixes radiogroup JS in p3p.xul
(Assignee)

Comment 2

13 years ago
Created attachment 159479 [details] [diff] [review]
Patch to venkman

Updates venkman-bpprops.js
(Assignee)

Updated

13 years ago
Attachment #159401 - Attachment description: Patch to cookies → Patch to cookie
(Assignee)

Comment 3

13 years ago
Created attachment 159625 [details] [diff] [review]
Patch to security

Updates editcerts.js, editemailcert.xul, editsslcert.xul, pref-masterpass.js
and pref-masterpass.xul
(Assignee)

Comment 4

13 years ago
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
(Assignee)

Comment 5

13 years ago
Created attachment 159652 [details] [diff] [review]
Patch to mail

Patches am-smime.js and pref-masterpass.xul in mail tree
(Assignee)

Comment 6

13 years ago
Created attachment 159653 [details] [diff] [review]
Patches to xpfe

Patches pref-applications-edit.xul
(Assignee)

Updated

13 years ago
Attachment #159652 - Flags: review?(mscott)
(Assignee)

Comment 7

13 years ago
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe

r+sr=me on attachment 159653 [details] [diff] [review] from Neil via IRC
Attachment #159653 - Flags: superreview+
Attachment #159653 - Flags: review+
(Assignee)

Comment 8

13 years ago
Comment on attachment 159625 [details] [diff] [review]
Patch to security

for moa
Attachment #159625 - Flags: review?(kaie)
(Assignee)

Updated

13 years ago
Attachment #159647 - Flags: review?(neil.parkwaycc.co.uk)
(Assignee)

Comment 9

13 years ago
Comment on attachment 159401 [details] [diff] [review]
Patch to cookie

r+sr=neil via IRC
Attachment #159401 - Flags: superreview+
Attachment #159401 - Flags: review+
Attachment #159401 - Flags: approval1.8a4?
(Assignee)

Comment 10

13 years ago
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe

Requesting approval for simple patch
Attachment #159653 - Flags: approval1.8a4?
(Assignee)

Updated

13 years ago
Attachment #159479 - Flags: review?(rginda)

Comment 11

13 years ago
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.
Attachment #159479 - Flags: review?(rginda) → review-

Comment 12

13 years ago
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe

a=asa for checkin to 1.8a4.
Attachment #159653 - Flags: approval1.8a4? → approval1.8a4+
(Assignee)

Comment 13

13 years ago
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

13 years ago
Comment on attachment 159401 [details] [diff] [review]
Patch to cookie

a=asa for 1.8a4 checkin.
Attachment #159401 - Flags: approval1.8a4? → approval1.8a4+
(Assignee)

Comment 15

13 years ago
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

13 years ago
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.
Attachment #159647 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Comment 17

13 years ago
Created attachment 160396 [details] [diff] [review]
Patch for mailnews v0.2

Takes on board Neil's comments, patch does not have askSendFormat.js changes
Attachment #159647 - Attachment is obsolete: true
(Assignee)

Comment 18

13 years ago
Comment on attachment 160396 [details] [diff] [review]
Patch for mailnews v0.2

Carrying forward r=
Attachment #160396 - Flags: superreview?(bienvenu)
Attachment #160396 - Flags: review+
(Assignee)

Comment 19

13 years ago
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.

Updated

13 years ago
Attachment #160396 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 20

13 years ago
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
(Assignee)

Updated

13 years ago
Attachment #160404 - Flags: review?(neil.parkwaycc.co.uk)

Comment 21

13 years ago
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.
Attachment #160404 - Flags: review?(neil.parkwaycc.co.uk) → review-
(Assignee)

Comment 22

13 years ago
Created attachment 162007 [details] [diff] [review]
Patch to askSendFormat v0.2

Uses parseInt to make param.action be an integer
Attachment #160404 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #162007 - Flags: review?(neil.parkwaycc.co.uk)

Comment 23

13 years ago
Comment on attachment 162007 [details] [diff] [review]
Patch to askSendFormat v0.2

I had hoped you'd change the caller ;-)
Attachment #162007 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Updated

13 years ago
Attachment #162007 - Flags: superreview?(bienvenu)

Comment 24

13 years ago
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
Attachment #162007 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 25

13 years ago
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

Updated

13 years ago
Attachment #159652 - Flags: review?(mscott)
(Assignee)

Comment 26

13 years ago
Comment on attachment 159652 [details] [diff] [review]
Patch to mail

Requesting r= on mail patch
Attachment #159652 - Flags: review?(mscott)

Updated

13 years ago
Attachment #159652 - Flags: review?(mscott) → review+
(Assignee)

Comment 27

13 years ago
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

12 years ago
Comment on attachment 159625 [details] [diff] [review]
Patch to security

Your change looks correct.

r=kaie assuming you have tested it still works.
Attachment #159625 - Flags: review?(kaie) → review+
(Assignee)

Comment 29

6 years ago
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.
Attachment #159625 - Attachment is obsolete: true
Attachment #537342 - Flags: review?(kaie)

Comment 30

6 years ago
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!
Attachment #537342 - Flags: review?(kaie) → review+

Comment 31

6 years ago
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!
(Assignee)

Updated

6 years ago
Attachment #537342 - Attachment description: Unbitrotted pki patch → Unbitrotted pki patch [Checked in: trunk comment 32]
(Assignee)

Comment 32

6 years ago
Comment on attachment 537342 [details] [diff] [review]
Unbitrotted pki patch [Checked in: trunk comment 32]

http://hg.mozilla.org/mozilla-central/rev/b7502e9fbd30
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.