Closed
Bug 260364
Opened 20 years ago
Closed 14 years ago
Fixup radiogroups to use new .value functionality of radio.xml
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: iannbugzilla, Assigned: iannbugzilla)
Details
Attachments
(7 files, 3 obsolete files)
1.59 KB,
patch
|
iannbugzilla
:
review+
iannbugzilla
:
superreview+
asa
:
approval1.8a4+
|
Details | Diff | Splinter Review |
1.98 KB,
patch
|
rginda
:
review-
|
Details | Diff | Splinter Review |
5.17 KB,
patch
|
mscott
:
review+
|
Details | Diff | Splinter Review |
2.34 KB,
patch
|
iannbugzilla
:
review+
iannbugzilla
:
superreview+
asa
:
approval1.8a4+
|
Details | Diff | Splinter Review |
15.53 KB,
patch
|
iannbugzilla
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
7.21 KB,
patch
|
neil
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
5.09 KB,
patch
|
KaiE
:
review+
|
Details | Diff | Splinter Review |
Now that the fix to bug 250817 has landed there are certain parts of the code
that could hopefully be simplified now.
Attachment #159401 -
Attachment description: Patch to cookies → Patch to cookie
Updates editcerts.js, editemailcert.xul, editsslcert.xul, pref-masterpass.js
and pref-masterpass.xul
Patches askSendFormat.js, importDialog.js, am-addressing.js, aw-server.js,
smtpEditOverlay.js, searchTermOverlay.js and am-smime.js
Attachment #159652 -
Flags: review?(mscott)
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+
Comment on attachment 159625 [details] [diff] [review]
Patch to security
for moa
Attachment #159625 -
Flags: review?(kaie)
Attachment #159647 -
Flags: review?(neil.parkwaycc.co.uk)
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•20 years ago
|
||
Comment on attachment 159653 [details] [diff] [review]
Patches to xpfe
Requesting approval for simple patch
Attachment #159653 -
Flags: approval1.8a4?
Attachment #159479 -
Flags: review?(rginda)
Comment 11•20 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•20 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•20 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•20 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•20 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•20 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•20 years ago
|
||
Takes on board Neil's comments, patch does not have askSendFormat.js changes
Attachment #159647 -
Attachment is obsolete: true
Assignee | ||
Comment 18•20 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•20 years ago
|
||
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•20 years ago
|
Attachment #160396 -
Flags: superreview?(bienvenu) → superreview+
Assignee | ||
Comment 20•20 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
Attachment #160404 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 21•20 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•20 years ago
|
||
Uses parseInt to make param.action be an integer
Attachment #160404 -
Attachment is obsolete: true
Attachment #162007 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 23•20 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+
Attachment #162007 -
Flags: superreview?(bienvenu)
Comment 24•20 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•20 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•20 years ago
|
Attachment #159652 -
Flags: review?(mscott)
Assignee | ||
Comment 26•20 years ago
|
||
Comment on attachment 159652 [details] [diff] [review]
Patch to mail
Requesting r= on mail patch
Attachment #159652 -
Flags: review?(mscott)
Updated•20 years ago
|
Attachment #159652 -
Flags: review?(mscott) → review+
Assignee | ||
Comment 27•20 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•20 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•14 years ago
|
||
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•14 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•14 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!
Attachment #537342 -
Attachment description: Unbitrotted pki patch → Unbitrotted pki patch [Checked in: trunk comment 32]
Assignee | ||
Comment 32•14 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
Status: ASSIGNED → RESOLVED
Closed: 14 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.
Description
•