Closed Bug 289444 Opened 16 years ago Closed 15 years ago

Give prefs panels titles a unique name

Categories

(SeaMonkey :: Preferences, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: prometeo.bugs, Assigned: prometeo.bugs)

References

(Blocks 1 open bug)

Details

(Keywords: fixed-seamonkey1.1a, fixed1.8.1)

Attachments

(6 files, 18 obsolete files)

70.38 KB, patch
prometeo.bugs
: review+
prometeo.bugs
: superreview+
chofmann
: approval1.8b2+
Details | Diff | Splinter Review
30.54 KB, patch
mnyromyr
: review+
Details | Diff | Splinter Review
14.15 KB, patch
glazou
: review+
Details | Diff | Splinter Review
15.26 KB, patch
prometeo.bugs
: review+
Details | Diff | Splinter Review
1.62 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
69.62 KB, patch
prometeo.bugs
: review+
prometeo.bugs
: superreview+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050407
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050407

Almost all pref-xxxx.dtd's use lHeader for the pane title. This is not good for
using entities in help docs.

Reproducible: Always

Steps to Reproduce:
Blocks: 287414
 > This is not good for
> using entities in help docs.

This can't be good from any point of view.
Status: UNCONFIRMED → NEW
Ever confirmed: true
--> Giacomo
Assignee: prefs → giacomo.magnini
Attachment #180263 - Flags: review?(kairo)
I noticed security.title is already in use as an entity, also just wondering
whether each one should be prefaced pref- ?
Comment on attachment 180263 [details] [diff] [review]
First cut: xpfe prefs. Comments welcome. Removed a few unused entities here and there, corrected and moved a localization note.

>+      headertitle="&advanced.title;">

Why not e.g. &pref.advanced.title; ?
As you might include quit some dtd files into one XHTML file for the planned
help work, it might better to minimize the chance of clashes and make the
entity names more "talking"...

Same for all those others (sorry)...

>Index: content/pref-http.xul

>+      headertitle="&httpNetworking.title;">

Where's the logic for adding "Networking" in the name?
Why not http.title (or even better pref.http.title, see above)?


>+<!ENTITY advanced.title               "Advanced">
> 
> <!ENTITY advancedTitle.label          "Features that help interpret web pages">

Would you mind changing that into pref.advanced.desc or similar, so that those
two entities don't sound like they're the same thing?

>+<!ENTITY  cache.title                   "Cache">
> <!ENTITY  cacheTitle.label              "Set Cache Options">

same here

>+<!ENTITY mouseWheel.title             "Mouse Wheel">
> <!ENTITY mouseWheelTitle.label        "Mouse wheel configuration">

again

>+<!ENTITY  proxies.title                 "Proxies">
> <!ENTITY  proxyTitle.label              "Configure Proxies to Access the Internet">

and again

>+<!ENTITY smartUpdate.title                    "Software Installation">
> <!ENTITY smartTitle.label                     "Manage Software Installations and Updates">

and again


I'd like to see a patch with those fixed before really r+ it... Even though
those might be nits...
Attachment #180263 - Flags: review?(kairo) → review-
(In reply to comment #5)
> >+<!ENTITY mouseWheel.title             "Mouse Wheel">
> > <!ENTITY mouseWheelTitle.label        "Mouse wheel configuration">
> 
> again

Removed the second entity as it is not used anywhere.

> >+<!ENTITY smartUpdate.title                    "Software Installation">
> > <!ENTITY smartTitle.label                     "Manage Software Installations
and Updates">
> 
> and again

No. smartTitle.label is the title of the groupbox, not a comment for
pref.smartUpdate.title.

All of the rest has been fixed.
Attached patch Second cut, nits fixed. (obsolete) — Splinter Review
Attachment #180263 - Attachment is obsolete: true
Attachment #182262 - Flags: review?(kairo)
Attachment #182262 - Attachment is obsolete: true
Attachment #182262 - Flags: review?(kairo)
Attachment #182268 - Flags: review?(kairo)
Comment on attachment 182268 [details] [diff] [review]
Two wrong entities fixed (I hate Kate editor!)

looks good to me. r=me given that all panels are tested to still work
correctly. Bonus points for some extra cleanup you did ;-)
Attachment #182268 - Flags: review?(kairo) → review+
Attachment #182268 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 182268 [details] [diff] [review]
Two wrong entities fixed (I hate Kate editor!)

Nit: pref-download.xul uses pref.downloads.title - please remove the 's' for
consistency.

<caption label> entity names should always end in .caption i.e.
&pref.advanced.caption; sr=me with this fixed.
Attachment #182268 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(In reply to comment #10)
> (From update of attachment 182268 [details] [diff] [review] [edit])
> Nit: pref-download.xul uses pref.downloads.title - please remove the 's' for
> consistency.

Done locally.

> <caption label> entity names should always end in .caption i.e.
> &pref.advanced.caption; sr=me with this fixed.

DO you mean to change only the entities touched here or all of the wrong named
entities spread across all of the files? 
Some additional cleanup (formatting basically). Fixed also a few .caption's
more than the ones noted (already done when Neil told me to fix only the ones
affected by my previous patch). A quick reread of the patch before asking a=
and checkin will be appreciated.
Attachment #182268 - Attachment is obsolete: true
Attachment #182315 - Flags: superreview+
Attachment #182315 - Flags: review+
Comment on attachment 182315 [details] [diff] [review]
sr nits addressed, carrying over flags

>Index: content/pref-winhooks.xul
>===================================================================
>@@ -46,17 +46,17 @@
> <!ENTITY % platformDTD SYSTEM "chrome://global-platform/locale/platformDialogOverlay.dtd" >
> %platformDTD;
> <!ENTITY % prefWinhooksDTD SYSTEM "chrome://communicator/locale/pref/pref-winhooks.dtd" >
> %prefWinhooksDTD;
> ]>
> 
> <page xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>       onload="parent.initPanel('chrome://communicator/content/pref/pref-winhooks.xul');"
>-      headertitle="&title.label;">
>+      headertitle="&pref.system.title;">
Shouldn't this be pref.winHooks.title ?
>Index: locale/en-US/pref-winhooks.dtd
>===================================================================
>@@ -1,33 +1,33 @@
> <!-- LOCALIZATION NOTE : FILE "Windows integration" preferences -->
> 
>-<!ENTITY title.label "System">
>+<!ENTITY pref.system.title    "System">

and again.
Very low risk patch affecting only Seamonkey pref panels.
Attachment #182315 - Attachment is obsolete: true
Attachment #182353 - Flags: superreview+
Attachment #182353 - Flags: review+
Attachment #182353 - Flags: approval1.8b2?
Comment on attachment 182353 [details] [diff] [review]
What Ian said. (Checked in)

a=chofmann
Attachment #182353 - Flags: approval1.8b2? → approval1.8b2+
Comment on attachment 182353 [details] [diff] [review]
What Ian said. (Checked in)

Checking in content/pref-advanced.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-advanced.xul
,v  <--  pref-advanced.xul
new revision: 1.89; previous revision: 1.88
done
Checking in content/pref-appearance.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-appearance.x
ul,v  <--  pref-appearance.xul
new revision: 1.59; previous revision: 1.58
done
Checking in content/pref-applications.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-applications
.xul,v	<--  pref-applications.xul
new revision: 1.56; previous revision: 1.55
done
Checking in content/pref-cache.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-cache.xul,v 
<--  pref-cache.xul
new revision: 1.61; previous revision: 1.60
done
Checking in content/pref-colors.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-colors.xul,v
 <--  pref-colors.xul
new revision: 1.58; previous revision: 1.57
done
Checking in content/pref-cookies.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-cookies.xul,
v  <--	pref-cookies.xul
new revision: 1.40; previous revision: 1.39
done
Checking in content/pref-download.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-download.xul
,v  <--  pref-download.xul
new revision: 1.38; previous revision: 1.37
done
Checking in content/pref-fonts.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-fonts.xul,v 
<--  pref-fonts.xul
new revision: 1.85; previous revision: 1.84
done
Checking in content/pref-history.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-history.xul,
v  <--	pref-history.xul
new revision: 1.31; previous revision: 1.30
done
Checking in content/pref-http.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-http.xul,v 
<--  pref-http.xul
new revision: 1.8; previous revision: 1.7
done
Checking in content/pref-images.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-images.xul,v
 <--  pref-images.xul
new revision: 1.14; previous revision: 1.13
done
Checking in content/pref-keynav.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-keynav.xul,v
 <--  pref-keynav.xul
new revision: 1.6; previous revision: 1.5
done
Checking in content/pref-mousewheel.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-mousewheel.x
ul,v  <--  pref-mousewheel.xul
new revision: 1.44; previous revision: 1.43
done
Checking in content/pref-navigator.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-navigator.xu
l,v  <--  pref-navigator.xul
new revision: 1.75; previous revision: 1.74
done
Checking in content/pref-policies.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-policies.xul
,v  <--  pref-policies.xul
new revision: 1.19; previous revision: 1.18
done
Checking in content/pref-popups.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-popups.xul,v
 <--  pref-popups.xul
new revision: 1.12; previous revision: 1.11
done
Checking in content/pref-proxies.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-proxies.xul,
v  <--	pref-proxies.xul
new revision: 1.59; previous revision: 1.58
done
Checking in content/pref-scripts.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-scripts.xul,
v  <--	pref-scripts.xul
new revision: 1.20; previous revision: 1.19
done
Checking in content/pref-search.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-search.xul,v
 <--  pref-search.xul
new revision: 1.37; previous revision: 1.36
done
Checking in content/pref-security.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-security.xul
,v  <--  pref-security.xul
new revision: 1.9; previous revision: 1.8
done
Checking in content/pref-smart_browsing.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-smart_browsi
ng.xul,v  <--  pref-smart_browsing.xul
new revision: 1.67; previous revision: 1.66
done
Checking in content/pref-smartupdate.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-smartupdate.
xul,v  <--  pref-smartupdate.xul
new revision: 1.46; previous revision: 1.45
done
Checking in content/pref-themes.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-themes.xul,v
 <--  pref-themes.xul
new revision: 1.56; previous revision: 1.55
done
Checking in content/pref-winhooks.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-winhooks.xul
,v  <--  pref-winhooks.xul
new revision: 1.34; previous revision: 1.33
done
Checking in locale/en-US/pref-advanced.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-advance
d.dtd,v  <--  pref-advanced.dtd
new revision: 1.29; previous revision: 1.28
done
Checking in locale/en-US/pref-appearance.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-appeara
nce.dtd,v  <--	pref-appearance.dtd
new revision: 1.21; previous revision: 1.20
done
Checking in locale/en-US/pref-applications.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-applica
tions.dtd,v  <--  pref-applications.dtd
new revision: 1.15; previous revision: 1.14
done
Checking in locale/en-US/pref-bidi.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-bidi.dt
d,v  <--  pref-bidi.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locale/en-US/pref-cache.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-cache.d
td,v  <--  pref-cache.dtd
new revision: 1.25; previous revision: 1.24
done
Checking in locale/en-US/pref-charset.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-charset
.dtd,v	<--  pref-charset.dtd
new revision: 1.7; previous revision: 1.6
done
Checking in locale/en-US/pref-colors.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-colors.
dtd,v  <--  pref-colors.dtd
new revision: 1.24; previous revision: 1.23
done
Checking in locale/en-US/pref-cookies.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-cookies
.dtd,v	<--  pref-cookies.dtd
new revision: 1.25; previous revision: 1.24
done
Checking in locale/en-US/pref-download.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-downloa
d.dtd,v  <--  pref-download.dtd
new revision: 1.18; previous revision: 1.17
done
Checking in locale/en-US/pref-fonts.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-fonts.d
td,v  <--  pref-fonts.dtd
new revision: 1.37; previous revision: 1.36
done
Checking in locale/en-US/pref-history.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-history
.dtd,v	<--  pref-history.dtd
new revision: 1.11; previous revision: 1.10
done
Checking in locale/en-US/pref-http.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-http.dt
d,v  <--  pref-http.dtd
new revision: 1.6; previous revision: 1.5
done
Checking in locale/en-US/pref-images.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-images.
dtd,v  <--  pref-images.dtd
new revision: 1.8; previous revision: 1.7
done
Checking in locale/en-US/pref-keynav.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-keynav.
dtd,v  <--  pref-keynav.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locale/en-US/pref-mousewheel.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-mousewh
eel.dtd,v  <--	pref-mousewheel.dtd
new revision: 1.12; previous revision: 1.11
done
Checking in locale/en-US/pref-navigator.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-navigat
or.dtd,v  <--  pref-navigator.dtd
new revision: 1.27; previous revision: 1.26
done
Checking in locale/en-US/pref-policies.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-policie
s.dtd,v  <--  pref-policies.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locale/en-US/pref-popups.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-popups.
dtd,v  <--  pref-popups.dtd
new revision: 1.7; previous revision: 1.6
done
Checking in locale/en-US/pref-proxies.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-proxies
.dtd,v	<--  pref-proxies.dtd
new revision: 1.25; previous revision: 1.24
done
Checking in locale/en-US/pref-scripts.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-scripts
.dtd,v	<--  pref-scripts.dtd
new revision: 1.11; previous revision: 1.10
done
Checking in locale/en-US/pref-search.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-search.
dtd,v  <--  pref-search.dtd
new revision: 1.21; previous revision: 1.20
done
Checking in locale/en-US/pref-security.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-securit
y.dtd,v  <--  pref-security.dtd
new revision: 1.7; previous revision: 1.6
done
Checking in locale/en-US/pref-smart_browsing.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-smart_b
rowsing.dtd,v  <--  pref-smart_browsing.dtd
new revision: 1.31; previous revision: 1.30
done
Checking in locale/en-US/pref-smartupdate.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-smartup
date.dtd,v  <--  pref-smartupdate.dtd
new revision: 1.13; previous revision: 1.12
done
Checking in locale/en-US/pref-themes.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-themes.
dtd,v  <--  pref-themes.dtd
new revision: 1.13; previous revision: 1.12
done
Checking in locale/en-US/pref-winhooks.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-winhook
s.dtd,v  <--  pref-winhooks.dtd
new revision: 1.15; previous revision: 1.14
done
Attachment #182353 - Attachment description: What Ian said. → What Ian said. (Checked in)
Attached patch MailNews part (obsolete) — Splinter Review
From a short glance over all of them, all those new four patches look good to
me... You might want to ask Neil for r+sr, that might be the fastest way to get
reviews :)
Comment on attachment 182391 [details] [diff] [review]
Security fixes: removed also a few unused entities in pref-security.dtd

>Index: manager/locales/en-US/chrome/pippki/pref-security.dtd
>===================================================================
>+<!ENTITY  ssl.label                "SSL">
> 
>-<!ENTITY  ssl.label              "SSL">
I cannot see this entity being used anywhere.
>-
>-<!ENTITY  certs.title            "Certificates">
>-<!ENTITY  certs.label            "Certificates">
>+<!ENTITY  pref.certs.title         "Certificates">
>+<!ENTITY  certs.label              "Certificates">
Maybe this file should be renamed to pref-certs.dtd and the certselect.* and
SSLClientAuthMethod.caption entities moved from pref-ssl.dtd into it, then
pref-certs.xul altered so that it only loads pref-certs.dtd instead of
pref-security.dtd and pref-ssl.dtd

>Index: manager/locales/en-US/chrome/pippki/pref-ssl.dtd
>===================================================================
>+<!ENTITY  SSLClientAuthMethod.caption       "Client Certificate Selection">
.
.
>+<!ENTITY certselect.description             "Decide how &brandShortName; selects a security certificate to present to web sites that require one:">
>+<!ENTITY certselect.auto                    "Select Automatically">
>+<!ENTITY certselect.ask                     "Ask Every Time">
Should be moved into pref-certs.dtd/pref-security.dtd

>Index: manager/pki/resources/content/pref-certs.xul
>===================================================================
>   <!ENTITY % prefSslDTD SYSTEM "chrome://pippki/locale/pref-ssl.dtd">
>   %brandDTD;
>   %prefSec;
>   %prefSslDTD;
> ]>
See above.

>     <radiogroup id="certSelection" orient="horizontal" preftype="string" 
> 	        prefstring="security.default_personal_cert">
>     <radio label="&certselect.auto;" value="Select Automatically"/>
>     <radio label="&certselect.ask;" value="Ask Every Time"/>
>     </radiogroup>
>   </groupbox>
Be nice to have some accesskeys for these radio buttons.

>Index: manager/pki/resources/content/pref-masterpass.xul
>===================================================================
>       <radiogroup id="passwordAskTimes"
>                   prefstring="security.ask_for_password"
>                   flex="1">
>         <!-- note that these values are different than what NSS uses, which
>              are (0, -1, 1) respectively -->
>         <radio value="0" label="&managepassword.askfirsttime;" id="askFirstTime"
Again be nice to have some accesskeys for these radio buttons.

>Index: manager/pki/resources/content/pref-ssl.xul
>===================================================================
>     <vbox flex="1" align="start">
>       <checkbox id="enableSSL2" label="&enable.ssl2;"
>                 prefstring="security.enable_ssl2"/>
>       <checkbox id="enableSSL3" label="&enable.ssl3;"
>                 prefstring="security.enable_ssl3"/>
>       <checkbox id="enableTLS" label="&enable.tls;"
Accesskeys for all the checkboxes in this groupbox.

>   <groupbox align="start">
>-    <caption label="&SSLWarnings;"/>
>+    <caption label="&SSLWarnings.caption;"/>
>     <description>&warn.description;</description>
>     <!-- Prefs -->
>     <checkbox id="warnEnteringSecure" label="&warn.enteringsecure;"
>               prefstring="security.warn_entering_secure"/>
>     <checkbox id="warnEnteringWeak" label="&warn.enteringweak;"
>               prefstring="security.warn_entering_weak"/>
>     <checkbox id="warnLeavingSecure" label="&warn.leavingsecure;"
>               prefstring="security.warn_leaving_secure"/>
Accesskeys for all the checkboxes in this groupbox.

>Index: manager/pki/resources/content/pref-validation.xul
>===================================================================
>     <radiogroup id="securityOCSPEnabled"
>                 prefstring="security.OCSP.enabled">
>         <radio value="0" label="&disableOCSP.label;" oncommand="doEnabling();"/>
>         <radio value="1" label="&certOCSP.label;" oncommand="doEnabling();"/>
>         <radio value="2" label="&proxyOCSP.label;" oncommand="doEnabling();"/>
> 
Again be nice to have some accesskeys for these radio buttons.
Comment on attachment 182389 [details] [diff] [review]
LP, inspector and wallet prefpanel fixes and minor cleanup

>Index: inspector/resources/content/prefs/pref-inspector.xul
>===================================================================
>   <vbox id="bxGeneralPrefs">
>     <groupbox id="tbxBlink" flex="1">
>-      <caption label="&elementBlinking.label;"/>
>+      <caption label="&elementBlinking.caption;"/>
>       <vbox>
>         <checkbox id="cbElOn" label="&blinkSelectedElement.label;" oncommand="enableBlinkPrefs(this.checked)"
>             preftype="bool" prefstring="inspector.blink.on" prefattribute="checked"/>
>         <grid>
>           <columns>
>             <column/>
>             <column flex="1"/>
>           </columns>
>@@ -100,17 +100,17 @@
>         <vbox align="start">
>           <checkbox id="cbElInvert" label="&invertColors.label;"
>                     preftype="bool" prefstring="inspector.blink.invert" prefattribute="checked"/>
>         </vbox>
>       </vbox>
>     </groupbox>
>     
>     <groupbox id="tbxSidebar" flex="1">
>-      <caption label="&Sidebar.label;"/>
>+      <caption label="&Sidebar.caption;"/>
>       <vbox flex="1">
>         <vbox id="bxSidebarInstall" flex="1">
>           <hbox>
>             <description id="txSidebarMsg" flex="1">&installHowTo.label;</description>
>           </hbox>
>           <button id="btnSidebarInstall" label="&install.label;" oncommand="sidebarPref.installSidebar()"/>
>         </vbox>
>       </vbox>
Some accesskeys would be nice to add whilst we're here.

>Index: wallet/resources/locale/en-US/pref-passwords.dtd
>===================================================================
> <!ENTITY  signonEnabled.label         "Remember passwords">
> <!ENTITY  signonEnabled.accesskey     "r">
Change this to "R"

> <!ENTITY  viewSignons.label           "Manage Stored Passwords">
> <!ENTITY  viewSignons.accesskey       "e">
Could be changed to "M"

>-<!ENTITY  encryptHeader.label         "Encrypting versus Obscuring">
>+<!ENTITY  encryptHeader.caption       "Encrypting versus Obscuring">
> <!ENTITY  encryptDescription.label    "Sensitive data that is stored on your hard disk can be encrypted to prevent the data from being read by an intruder.  A password is used to access the data.">
> <!ENTITY  encryptEnabled.label        "Use encryption when storing sensitive data.">
> <!ENTITY  encryptEnabled.accesskey    "n">
Could be changed to "U"

> <!ENTITY  encryptExpire.label         "Expire master password after each use.">
> <!ENTITY  encryptExpire.accesskey     "x">
These two entities do not seem to be used and if they are needed for the future
then probably best to be in pref-masterpass.dtd not here.
If Neil agrees, I will spin off a bug for reorganizing security related dtd/xul
files, and another one to add a bunch of accesskeys...
Attachment #182388 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182388 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182389 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182389 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182390 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182390 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182391 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182391 - Flags: review?(neil.parkwaycc.co.uk)
Small update: I've made locally all the corrections IanN suggested but the dtd
reorganizing, as Neil pointed out that those files maybe used by FX as well, so
they'll need a bug on their own. Should I update the patches here or wait for
Neil's r/sr (which will take a long time)?
Yes I would spin off the reorganisation bits into another bug.

As for the security patch:
pref-masterpass.dtd is only used by pref-masterpass.xul which in turn is used by
PrefOverlay.xul which overlays preftree.xul

pref-security.dtd is only used by pref-certs.xul which in turn is used by
PrefOverlay.xul which overlays preftree.xul

pref-ssl.dtd is only used by cipherinfo.xul, pref-certs.xul, pref-ciphers.xul,
pref-ssl.xul, ssl2ciphers.xul, ssl3tlsciphers.xul and ssl3tlsciphers2.xul
cipherinfo.xul is used by pref-ciphers.js
pref-ciphers.js, ssl2ciphers.xul, ssl3tlsciphers.xul and ssl3tlsciphers2.xul are
used by pref-ciphers.xul
pref-ciphers.xul is used by pref-ssl.xul
pref-certs.xul and pref-ssl.xul are used by PrefOverlay.xul which overlays
preftree.xul

pref-validation.dtd is used by crlImportDialog.xul, crlManager.xul,
pref-crlupdate.xul and pref-validation.xul
crlImportDialog.xul is used by nsNSSDialogs.cpp
crlManager.xul is used by pref-validation.js, advanced.js and privacy.js
pref-crlupdate.xul is used by crlImportDialog.js and crlManager.js
pref-validation.xul is used by PrefOverlay.xul which overlays preftree.xul

So out of all the files only pref-validation.dtd seems to be used by FF/TB.
Attachment #182390 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182390 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182389 - Attachment is obsolete: true
Attachment #182389 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182389 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182388 - Attachment is obsolete: true
Attachment #182388 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182388 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182391 - Attachment is obsolete: true
Attachment #182391 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #182391 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #182390 - Attachment is obsolete: true
Attachment #189520 - Flags: review?(bugzilla)
Attachment #189517 - Flags: review?(mnyromyr)
Attachment #189517 - Flags: review?(mnyromyr) → review+
Attachment #189517 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

>+<!ENTITY pref.ldap.window.title    "LDAP Directory Servers">
Where does the "window" fit in? Or should this just be pref.ldap.title?

Won't the Thunderbird locales be affected?
Attachment #189517 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(In reply to comment #31)
> (From update of attachment 189517 [details] [diff] [review] [edit])
> >+<!ENTITY pref.ldap.window.title    "LDAP Directory Servers">
> Where does the "window" fit in? Or should this just be pref.ldap.title?

Will remove that.

> Won't the Thunderbird locales be affected?
 
Hmmm, I think you're right. Who should I ask advice to, mscott?
(In reply to comment #32)
>>Won't the Thunderbird locales be affected?
>Hmmm, I think you're right. Who should I ask advice to, mscott?
Wait, I think I was confusing prefs with account settings (which are shared).
(In reply to comment #33)
> Wait, I think I was confusing prefs with account settings (which are shared).

Well, smontagu told me over IRC that pref-editdirectories.xul uses
pref-directory.dtd in TB. By looking at lxr, seems like these files have 2
versions, one in mailnews and the other in mail. Anyway, if everything looks
good, someone should check this in... :)
Attachment #189523 - Flags: review?(timeless)
Comment on attachment 189520 [details] [diff] [review]
Lp, DOMI, wallet: updated patch including requested accesskeys.

>Index: inspector/resources/content/prefs/pref-inspector.xul
>===================================================================
>@@ -49,20 +49,20 @@
>           document.getElementById(els[i]).setAttribute("disabled", true);
>       }
>     }
>     
>   ]]></script>
> 
>   <vbox id="bxGeneralPrefs">
>     <groupbox id="tbxBlink" flex="1">
>-      <caption label="&elementBlinking.label;"/>
>+      <caption label="&elementBlinking.caption;"/>
>       <vbox>
>-        <checkbox id="cbElOn" label="&blinkSelectedElement.label;" oncommand="enableBlinkPrefs(this.checked)"
>-            preftype="bool" prefstring="inspector.blink.on" prefattribute="checked"/>
>+        <checkbox id="cbElOn" label="&blinkSelectedElement.label;" accesskey="&blinkSelectedElement.accesskey;"
>+	  oncommand="enableBlinkPrefs(this.checked)" preftype="bool" prefstring="inspector.blink.on" prefattribute="checked"/>
Second line needs to be indented by one more space

>Index: inspector/resources/locale/en-US/prefs.dtd
>@@ -1,14 +1,17 @@
.
.
> <!ENTITY invertColors.label "Invert Colors">
>+<!ENTITY invertColors.accesskey "I">
Choosing "C" instead of "I" might make things clearer.

r= with those changes
Attachment #189520 - Flags: review?(bugzilla) → review+
Comment on attachment 189522 [details] [diff] [review]
Updated patch for security/manager/, added some accesskeys as requested

>Index: manager/locales/en-US/chrome/pippki/pref-masterpass.dtd
>===================================================================
>@@ -31,25 +31,25 @@
.
.
>+<!ENTITY  pref.masterpass.title        "Master Passwords">
>+<!ENTITY  managepassword.caption       "Master Password Timeout">
>+<!ENTITY  managepassword.text          "&brandShortName; will ask for your master password:">
>+<!ENTITY  managepassword.askfirsttime  "The first time it is needed">
>+<!ENTITY  managepassword.askeverytime  "Every time it is needed">
>+<!ENTITY  managepassword.asktimeout    "If it has not been used for ">
>+<!ENTITY  managepassword.timeout.unit  "minutes or longer">
Still missing accesskeys for the above radio buttons (avoid "I" if you can).

>Index: manager/locales/en-US/chrome/pippki/pref-ssl.dtd
>===================================================================
>@@ -31,91 +31,91 @@
.
.
>+<!ENTITY pref.ssl.title                     "Secure Sockets Layer (SSL)">
>+<!ENTITY enable.ssl2                        "Enable SSL version 2">
>+<!ENTITY enable.ssl3                        "Enable SSL version 3">
>+<!ENTITY enable.tls                         "Enable TLS">
Accesskeys still required for these checkboxes.

>+
>+<!ENTITY edit.sslciphers                    "Edit Ciphers...">
>+<!ENTITY edit.sslciphers.accesskey          "E">
>+
>+<!ENTITY warn.description                   "&brandShortName; can alert you to the security status of the web page you are viewing. Set &brandShortName; to show a warning and ask permission before:">
>+<!ENTITY warn.enteringsecure                "Loading a page that supports encryption">
>+<!ENTITY warn.enteringweak                  "Loading a page that uses low-grade encryption">
>+<!ENTITY warn.insecurepost                  "Sending form data from an unencrypted page to an unencrypted page">
>+<!ENTITY warn.leavingsecure                 "Leaving a page that supports encryption">
>+<!ENTITY warn.secureredirect                "Redirecting from one encrypted page to another">
>+<!ENTITY warn.secureredirecttoinsecure      "Redirecting from an encrypted page to an unencrypted page">
>+<!ENTITY warn.viewmixed                     "Viewing a page with an encrypted/unencrypted mix">
Again accesskeys still required for these checkboxes.

>+
>+<!ENTITY certselect.description             "Decide how &brandShortName; selects a security certificate to present to web sites that require one:">
>+<!ENTITY certselect.auto                    "Select Automatically">
>+<!ENTITY certselect.ask                     "Ask Every Time">
Again accesskeys still required for these radio buttons.

> 
> <!-- Cipher pref window -->
>-<!ENTITY cipher.title "SSL: Edit Ciphers">
>-<!ENTITY cipher.ssl2.label "SSL2 Cipher Suites">
>-<!ENTITY cipher.ssl3.label "SSL3/TLS Cipher Suites">
>-<!ENTITY cipher.ssl3_extra.label "Extra SSL3/TLS Cipher Suites">
>-<!ENTITY cipher.tab.ssl2 "SSL2">
>-<!ENTITY cipher.tab.ssl3tls "SSL3/TLS">
>-<!ENTITY cipher.tab.ssl3tls_extra "Extra SSL3/TLS">
>+<!ENTITY cipher.title                       "SSL: Edit Ciphers">
>+<!ENTITY cipher.ssl2.label                  "SSL2 Cipher Suites">
>+<!ENTITY cipher.ssl3.label                  "SSL3/TLS Cipher Suites">
>+<!ENTITY cipher.ssl3_extra.label            "Extra SSL3/TLS Cipher Suites">
>+<!ENTITY cipher.tab.ssl2                    "SSL2">
>+<!ENTITY cipher.tab.ssl3tls                 "SSL3/TLS">
>+<!ENTITY cipher.tab.ssl3tls_extra           "Extra SSL3/TLS">
> 
> <!-- SSL2 Ciphers -->
> 
>-<!ENTITY cipher.ssl2.rc4_128 "128-bit RC4 encryption with RSA and an MD5 MAC">
>-<!ENTITY cipher.ssl2.rc2_128 "128-bit RC2 encryption with RSA and an MD5 MAC">
>-<!ENTITY cipher.ssl2.des_ede3_192 "168-bit Triple DES encryption with RSA and MD5 MAC ">
>-<!ENTITY cipher.ssl2.des_64 "56-bit DES encryption with RSA and an MD5 MAC">
>-<!ENTITY cipher.ssl2.rc4_40 "40-bit RC4 encryption with RSA and an MD5 MAC (export)">
>-<!ENTITY cipher.ssl2.rc2_40 "40-bit RC2 encryption with RSA and an MD5 MAC (export)">
>+<!ENTITY cipher.ssl2.rc4_128                "128-bit RC4 encryption with RSA and an MD5 MAC">
>+<!ENTITY cipher.ssl2.rc2_128                "128-bit RC2 encryption with RSA and an MD5 MAC">
>+<!ENTITY cipher.ssl2.des_ede3_192           "168-bit Triple DES encryption with RSA and MD5 MAC ">
>+<!ENTITY cipher.ssl2.des_64                 "56-bit DES encryption with RSA and an MD5 MAC">
>+<!ENTITY cipher.ssl2.rc4_40                 "40-bit RC4 encryption with RSA and an MD5 MAC (export)">
>+<!ENTITY cipher.ssl2.rc2_40                 "40-bit RC2 encryption with RSA and an MD5 MAC (export)">
> 
> <!-- SSL3/TLS ciphers -->
>-<!ENTITY cipher.ssl3.rsa_rc4_128_md5 "128-bit RC4 encryption with RSA and an MD5 MAC">
>-<!ENTITY cipher.ssl3.rsa_rc4_128_sha "128-bit RC4 encryption with RSA and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_fips_des_ede3_sha "168-bit Triple DES with RSA and a SHA1 MAC (FIPS)">
>-<!ENTITY cipher.ssl3.rsa_des_ede3_sha "168-bit Triple DES with RSA and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_fips_des_sha "56-bit DES encryption with RSA and a SHA1 MAC (FIPS)">
>-<!ENTITY cipher.ssl3.rsa_des_sha "56-bit DES encryption with RSA and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_1024_rc4_56_sha "56-bit RC4 encryption with RSA and a SHA1 MAC (export)">
>-<!ENTITY cipher.ssl3.rsa_1024_des_cbc_sha "56-bit DES encryption with RSA and a SHA1 MAC (export)">
>-<!ENTITY cipher.ssl3.rsa_rc4_40_md5 "40-bit RC4 encryption with RSA and an MD5 MAC (export)">
>-<!ENTITY cipher.ssl3.rsa_rc2_40_md5 "40-bit RC2 encryption with RSA and an MD5 MAC (export)">
>+<!ENTITY cipher.ssl3.rsa_rc4_128_md5        "128-bit RC4 encryption with RSA and an MD5 MAC">
>+<!ENTITY cipher.ssl3.rsa_rc4_128_sha        "128-bit RC4 encryption with RSA and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_fips_des_ede3_sha  "168-bit Triple DES with RSA and a SHA1 MAC (FIPS)">
>+<!ENTITY cipher.ssl3.rsa_des_ede3_sha       "168-bit Triple DES with RSA and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_fips_des_sha       "56-bit DES encryption with RSA and a SHA1 MAC (FIPS)">
>+<!ENTITY cipher.ssl3.rsa_des_sha            "56-bit DES encryption with RSA and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_1024_rc4_56_sha    "56-bit RC4 encryption with RSA and a SHA1 MAC (export)">
>+<!ENTITY cipher.ssl3.rsa_1024_des_cbc_sha   "56-bit DES encryption with RSA and a SHA1 MAC (export)">
>+<!ENTITY cipher.ssl3.rsa_rc4_40_md5         "40-bit RC4 encryption with RSA and an MD5 MAC (export)">
>+<!ENTITY cipher.ssl3.rsa_rc2_40_md5         "40-bit RC2 encryption with RSA and an MD5 MAC (export)">
> 
> <!-- Extra SSL3/TLS ciphers -->
>-<!ENTITY cipher.ssl3.dhe_rsa_aes_256_sha "256-bit AES encryption with RSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_dss_aes_256_sha "256-bit AES encryption with DSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_aes_256_sha "256-bit AES encryption with RSA and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_rsa_aes_128_sha "128-bit AES encryption with RSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_dss_aes_128_sha "128-bit AES encryption with DSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_aes_128_sha "128-bit AES encryption with RSA and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_rsa_des_ede3_sha "168-bit Triple DES with RSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_dss_des_ede3_sha "168-bit Triple DES with DSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_rsa_des_sha "56-bit DES encryption with RSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.dhe_dss_des_sha "56-bit DES encryption with DSA, DHE, and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_null_sha "No encryption with RSA authentication and a SHA1 MAC">
>-<!ENTITY cipher.ssl3.rsa_null_md5 "No encryption with RSA authentication and an MD5 MAC">
>+<!ENTITY cipher.ssl3.dhe_rsa_aes_256_sha    "256-bit AES encryption with RSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_dss_aes_256_sha    "256-bit AES encryption with DSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_aes_256_sha        "256-bit AES encryption with RSA and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_rsa_aes_128_sha    "128-bit AES encryption with RSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_dss_aes_128_sha    "128-bit AES encryption with DSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_aes_128_sha        "128-bit AES encryption with RSA and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_rsa_des_ede3_sha   "168-bit Triple DES with RSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_dss_des_ede3_sha   "168-bit Triple DES with DSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_rsa_des_sha        "56-bit DES encryption with RSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.dhe_dss_des_sha        "56-bit DES encryption with DSA, DHE, and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_null_sha           "No encryption with RSA authentication and a SHA1 MAC">
>+<!ENTITY cipher.ssl3.rsa_null_md5           "No encryption with RSA authentication and an MD5 MAC">

Not sure how we're going to deal with access keys for all these checkboxes and
buttons, any ideas? Possibly just not do it.

>Index: manager/locales/en-US/chrome/pippki/pref-validation.dtd
>===================================================================
>@@ -30,50 +30,50 @@
.
.
>+<!ENTITY  validation.deletecrl.label              "Delete">
>+<!ENTITY  validation.updatecrl.label              "Update">
>+<!ENTITY  validation.advanced.label               "Settings">
Accesskeys needed for these buttons and Import button, but see note at end.

>+<!ENTITY validation.ocsp.caption                  "OCSP">
>+<!ENTITY validation.ocsp.description              "&brandShortName; can use Online Certificate Status Protocol (OCSP) to verify certificates. Set &brandShortName; to use OCSP as follows:">
>+<!ENTITY disableOCSP.label                        "Do not use OCSP for certificate validation">
>+<!ENTITY certOCSP.label                           "Use OCSP to validate only certificates that specify an OCSP service URL">
>+<!ENTITY proxyOCSP.label                          "Use OCSP to validate all certificates using this URL and signer:">
Accesskeys needed for these radio buttons.

Some of the other dialog boxes brought up by clicking on buttons within the
pref panels above also require accesskeys:
Manage Stored Passwords
Manage Certificates
Manage Security Devices

Those accesskeys changes probably need to be spun off into a separate bug along
with maybe the Manage CRLs accesskeys noted above.

In addtion there are still some other pref panels with missing accesskeys e.g.:
Navigator (on Windows)
Helper Applications
Smart Browsing
Scripts & Plugins (perhaps)
Software Installation (case changes only e->E, c->C)
Debug (and it's tree)
Attachment #189522 - Flags: review?(bugzilla) → review-
(In reply to comment #36)
> (From update of attachment 189522 [details] [diff] [review] [edit])
> >Index: manager/locales/en-US/chrome/pippki/pref-masterpass.dtd
> >===================================================================
> >@@ -31,25 +31,25 @@
> .
> .
> >+<!ENTITY  pref.masterpass.title        "Master Passwords">
> >+<!ENTITY  managepassword.caption       "Master Password Timeout">
> >+<!ENTITY  managepassword.text          "&brandShortName; will ask for your
master password:">
> >+<!ENTITY  managepassword.askfirsttime  "The first time it is needed">
> >+<!ENTITY  managepassword.askeverytime  "Every time it is needed">
> >+<!ENTITY  managepassword.asktimeout    "If it has not been used for ">
> >+<!ENTITY  managepassword.timeout.unit  "minutes or longer">
> Still missing accesskeys for the above radio buttons (avoid "I" if you can).

Will add them.

> >Index: manager/locales/en-US/chrome/pippki/pref-ssl.dtd
> >===================================================================
> >@@ -31,91 +31,91 @@
> .
> .
> >+<!ENTITY pref.ssl.title                     "Secure Sockets Layer (SSL)">
> >+<!ENTITY enable.ssl2                        "Enable SSL version 2">
> >+<!ENTITY enable.ssl3                        "Enable SSL version 3">
> >+<!ENTITY enable.tls                         "Enable TLS">
> >+<!ENTITY edit.sslciphers                    "Edit Ciphers...">
> >+<!ENTITY edit.sslciphers.accesskey          "E">
> >+<!ENTITY warn.description                   "&brandShortName; can alert you
to the security status of the web page you are viewing. Set &brandShortName; to
show a warning and ask permission before:">
> >+<!ENTITY warn.enteringsecure                "Loading a page that supports
encryption">
> >+<!ENTITY warn.enteringweak                  "Loading a page that uses
low-grade encryption">
> >+<!ENTITY warn.insecurepost                  "Sending form data from an
unencrypted page to an unencrypted page">
> >+<!ENTITY warn.leavingsecure                 "Leaving a page that supports
encryption">
> >+<!ENTITY warn.secureredirect                "Redirecting from one encrypted
page to another">
> >+<!ENTITY warn.secureredirecttoinsecure      "Redirecting from an encrypted
page to an unencrypted page">
> >+<!ENTITY warn.viewmixed                     "Viewing a page with an
encrypted/unencrypted mix">
> >+<!ENTITY certselect.description             "Decide how &brandShortName;
selects a security certificate to present to web sites that require one:">
> >+<!ENTITY certselect.auto                    "Select Automatically">
> >+<!ENTITY certselect.ask                     "Ask Every Time">

They are there already (in the patch!).

> > <!-- Cipher pref window -->
> >-<!ENTITY cipher.title "SSL: Edit Ciphers">
> >-<!ENTITY cipher.ssl2.label "SSL2 Cipher Suites">
> >-<!ENTITY cipher.ssl3.label "SSL3/TLS Cipher Suites">
> >-<!ENTITY cipher.ssl3_extra.label "Extra SSL3/TLS Cipher Suites">
> >-<!ENTITY cipher.tab.ssl2 "SSL2">
> >-<!ENTITY cipher.tab.ssl3tls "SSL3/TLS">
> >-<!ENTITY cipher.tab.ssl3tls_extra "Extra SSL3/TLS">
> >+<!ENTITY cipher.title                       "SSL: Edit Ciphers">
> >+<!ENTITY cipher.ssl2.label                  "SSL2 Cipher Suites">
> >+<!ENTITY cipher.ssl3.label                  "SSL3/TLS Cipher Suites">
> >+<!ENTITY cipher.ssl3_extra.label            "Extra SSL3/TLS Cipher Suites">
> >+<!ENTITY cipher.tab.ssl2                    "SSL2">
> >+<!ENTITY cipher.tab.ssl3tls                 "SSL3/TLS">
> >+<!ENTITY cipher.tab.ssl3tls_extra           "Extra SSL3/TLS">
> > 
> > <!-- SSL2 Ciphers -->
> > 
> >-<!ENTITY cipher.ssl2.rc4_128 "128-bit RC4 encryption with RSA and an MD5 MAC">
> >-<!ENTITY cipher.ssl2.rc2_128 "128-bit RC2 encryption with RSA and an MD5 MAC">
> >-<!ENTITY cipher.ssl2.des_ede3_192 "168-bit Triple DES encryption with RSA
and MD5 MAC ">
> >-<!ENTITY cipher.ssl2.des_64 "56-bit DES encryption with RSA and an MD5 MAC">
> >-<!ENTITY cipher.ssl2.rc4_40 "40-bit RC4 encryption with RSA and an MD5 MAC
(export)">
> >-<!ENTITY cipher.ssl2.rc2_40 "40-bit RC2 encryption with RSA and an MD5 MAC
(export)">
> >+<!ENTITY cipher.ssl2.rc4_128                "128-bit RC4 encryption with RSA
and an MD5 MAC">
> >+<!ENTITY cipher.ssl2.rc2_128                "128-bit RC2 encryption with RSA
and an MD5 MAC">
> >+<!ENTITY cipher.ssl2.des_ede3_192           "168-bit Triple DES encryption
with RSA and MD5 MAC ">
> >+<!ENTITY cipher.ssl2.des_64                 "56-bit DES encryption with RSA
and an MD5 MAC">
> >+<!ENTITY cipher.ssl2.rc4_40                 "40-bit RC4 encryption with RSA
and an MD5 MAC (export)">
> >+<!ENTITY cipher.ssl2.rc2_40                 "40-bit RC2 encryption with RSA
and an MD5 MAC (export)">
> > 
> > <!-- SSL3/TLS ciphers -->
> >-<!ENTITY cipher.ssl3.rsa_rc4_128_md5 "128-bit RC4 encryption with RSA and an
MD5 MAC">
> >-<!ENTITY cipher.ssl3.rsa_rc4_128_sha "128-bit RC4 encryption with RSA and a
SHA1 MAC">
> >-<!ENTITY cipher.ssl3.rsa_fips_des_ede3_sha "168-bit Triple DES with RSA and
a SHA1 MAC (FIPS)">
> >-<!ENTITY cipher.ssl3.rsa_des_ede3_sha "168-bit Triple DES with RSA and a
SHA1 MAC">
> >-<!ENTITY cipher.ssl3.rsa_fips_des_sha "56-bit DES encryption with RSA and a
SHA1 MAC (FIPS)">
> >-<!ENTITY cipher.ssl3.rsa_des_sha "56-bit DES encryption with RSA and a SHA1
MAC">
> >-<!ENTITY cipher.ssl3.rsa_1024_rc4_56_sha "56-bit RC4 encryption with RSA and
a SHA1 MAC (export)">
> >-<!ENTITY cipher.ssl3.rsa_1024_des_cbc_sha "56-bit DES encryption with RSA
and a SHA1 MAC (export)">
> >-<!ENTITY cipher.ssl3.rsa_rc4_40_md5 "40-bit RC4 encryption with RSA and an
MD5 MAC (export)">
> >-<!ENTITY cipher.ssl3.rsa_rc2_40_md5 "40-bit RC2 encryption with RSA and an
MD5 MAC (export)">
> >+<!ENTITY cipher.ssl3.rsa_rc4_128_md5        "128-bit RC4 encryption with RSA
and an MD5 MAC">
> >+<!ENTITY cipher.ssl3.rsa_rc4_128_sha        "128-bit RC4 encryption with RSA
and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_fips_des_ede3_sha  "168-bit Triple DES with RSA and
a SHA1 MAC (FIPS)">
> >+<!ENTITY cipher.ssl3.rsa_des_ede3_sha       "168-bit Triple DES with RSA and
a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_fips_des_sha       "56-bit DES encryption with RSA
and a SHA1 MAC (FIPS)">
> >+<!ENTITY cipher.ssl3.rsa_des_sha            "56-bit DES encryption with RSA
and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_1024_rc4_56_sha    "56-bit RC4 encryption with RSA
and a SHA1 MAC (export)">
> >+<!ENTITY cipher.ssl3.rsa_1024_des_cbc_sha   "56-bit DES encryption with RSA
and a SHA1 MAC (export)">
> >+<!ENTITY cipher.ssl3.rsa_rc4_40_md5         "40-bit RC4 encryption with RSA
and an MD5 MAC (export)">
> >+<!ENTITY cipher.ssl3.rsa_rc2_40_md5         "40-bit RC2 encryption with RSA
and an MD5 MAC (export)">
> > 
> > <!-- Extra SSL3/TLS ciphers -->
> >-<!ENTITY cipher.ssl3.dhe_rsa_aes_256_sha "256-bit AES encryption with RSA,
DHE, and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_dss_aes_256_sha "256-bit AES encryption with DSA,
DHE, and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.rsa_aes_256_sha "256-bit AES encryption with RSA and a
SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_rsa_aes_128_sha "128-bit AES encryption with RSA,
DHE, and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_dss_aes_128_sha "128-bit AES encryption with DSA,
DHE, and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.rsa_aes_128_sha "128-bit AES encryption with RSA and a
SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_rsa_des_ede3_sha "168-bit Triple DES with RSA, DHE,
and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_dss_des_ede3_sha "168-bit Triple DES with DSA, DHE,
and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_rsa_des_sha "56-bit DES encryption with RSA, DHE,
and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.dhe_dss_des_sha "56-bit DES encryption with DSA, DHE,
and a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.rsa_null_sha "No encryption with RSA authentication and
a SHA1 MAC">
> >-<!ENTITY cipher.ssl3.rsa_null_md5 "No encryption with RSA authentication and
an MD5 MAC">
> >+<!ENTITY cipher.ssl3.dhe_rsa_aes_256_sha    "256-bit AES encryption with
RSA, DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_dss_aes_256_sha    "256-bit AES encryption with
DSA, DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_aes_256_sha        "256-bit AES encryption with RSA
and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_rsa_aes_128_sha    "128-bit AES encryption with
RSA, DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_dss_aes_128_sha    "128-bit AES encryption with
DSA, DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_aes_128_sha        "128-bit AES encryption with RSA
and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_rsa_des_ede3_sha   "168-bit Triple DES with RSA,
DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_dss_des_ede3_sha   "168-bit Triple DES with DSA,
DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_rsa_des_sha        "56-bit DES encryption with RSA,
DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.dhe_dss_des_sha        "56-bit DES encryption with DSA,
DHE, and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_null_sha           "No encryption with RSA
authentication and a SHA1 MAC">
> >+<!ENTITY cipher.ssl3.rsa_null_md5           "No encryption with RSA
authentication and an MD5 MAC">
> 
> Not sure how we're going to deal with access keys for all these checkboxes and
> buttons, any ideas? Possibly just not do it.

Adding them (at least for the ssl2 panel is possible). The problem I see is how
to add an acesskey for the Details button. And what is the influence on FF?
Still looking for a good solution on the MailNews patch, which seems to be
interacting with TB... Probably this will make much more sense once we
reorganize the various passwords/certificates files, all in one shot...


> >Index: manager/locales/en-US/chrome/pippki/pref-validation.dtd
> >===================================================================
> >@@ -30,50 +30,50 @@
> .
> .
> >+<!ENTITY  validation.deletecrl.label              "Delete">
> >+<!ENTITY  validation.updatecrl.label              "Update">
> >+<!ENTITY  validation.advanced.label               "Settings">
> Accesskeys needed for these buttons and Import button, but see note at end.

Will add them.

> >+<!ENTITY validation.ocsp.caption                  "OCSP">
> >+<!ENTITY validation.ocsp.description              "&brandShortName; can use
Online Certificate Status Protocol (OCSP) to verify certificates. Set
&brandShortName; to use OCSP as follows:">
> >+<!ENTITY disableOCSP.label                        "Do not use OCSP for
certificate validation">
> >+<!ENTITY certOCSP.label                           "Use OCSP to validate only
certificates that specify an OCSP service URL">
> >+<!ENTITY proxyOCSP.label                          "Use OCSP to validate all
certificates using this URL and signer:">
> Accesskeys needed for these radio buttons.

Already there!

> Some of the other dialog boxes brought up by clicking on buttons within the
> pref panels above also require accesskeys:
> Manage Stored Passwords
> Manage Certificates
> Manage Security Devices
> 
> Those accesskeys changes probably need to be spun off into a separate bug along
> with maybe the Manage CRLs accesskeys noted above.

Would you mind opening a bug on this and assigning it to me?

> In addtion there are still some other pref panels with missing accesskeys e.g.:
> Navigator (on Windows)
> Helper Applications
> Smart Browsing
> Scripts & Plugins (perhaps)
> Software Installation (case changes only e->E, c->C)
> Debug (and it's tree)

Would you mind opening one or more bugs on this and assigning it/them to me?
Please note that adding accesskey to Debug is useless, since we're not going to
ship a real product with such panel active...

Attachment #190822 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(In reply to comment #38)
> (In reply to comment #36)
> > (From update of attachment 189522 [details] [diff] [review] [edit] [edit])
> > >+<!ENTITY certselect.description             "Decide how &brandShortName;
> selects a security certificate to present to web sites that require one:">
> > >+<!ENTITY certselect.auto                    "Select Automatically">
> > >+<!ENTITY certselect.ask                     "Ask Every Time">
> 
> They are there already (in the patch!).
     <radio label="&certselect.auto;" value="Select Automatically"/>
     <radio label="&certselect.ask;" value="Ask Every Time"/>
I see no accesskey here.

> > Not sure how we're going to deal with access keys for all these checkboxes and
> > buttons, any ideas? Possibly just not do it.
> 
> Adding them (at least for the ssl2 panel is possible). The problem I see is how
> to add an acesskey for the Details button. And what is the influence on FF?
> Still looking for a good solution on the MailNews patch, which seems to be
> interacting with TB... Probably this will make much more sense once we
> reorganize the various passwords/certificates files, all in one shot...
Yes probably best to wait until then? Is there a bug for that?

> > >+<!ENTITY disableOCSP.label                        "Do not use OCSP for
> certificate validation">
> > >+<!ENTITY certOCSP.label                           "Use OCSP to validate only
> certificates that specify an OCSP service URL">
> > >+<!ENTITY proxyOCSP.label                          "Use OCSP to validate all
> certificates using this URL and signer:">
> > Accesskeys needed for these radio buttons.
> 
> Already there!
         <radio value="0" label="&disableOCSP.label;" oncommand="doEnabling();"/>
         <radio value="1" label="&certOCSP.label;" oncommand="doEnabling();"/>
         <radio value="2" label="&proxyOCSP.label;" oncommand="doEnabling();"/>
Again I see no accesskeys.

Will spin those two bugs off for you.
Blocks: 302477
Blocks: 302479
Attachment #189522 - Attachment is obsolete: true
Attachment #191316 - Flags: review?(bugzilla)
Comment on attachment 191316 [details] [diff] [review]
Hopefully the right patch this round... (security/manager/)

>Index: locales/en-US/chrome/pippki/pref-ssl.dtd
>===================================================================
>@@ -31,91 +31,101 @@
.
.
>+<!ENTITY certselect.description             "Decide how &brandShortName; selects a security certificate to present to web sites that require one:">
>+<!ENTITY certselect.auto                    "Select Automatically">
>+<!ENTITY certselect.auto.accesskey          "u">
>+<!ENTITY certselect.ask                     "Ask Every Time">
>+<!ENTITY certselect.ask.accesskey           "k">
e and A or A and E are available, why not use on of those combinations instead
of u and k?

>Index: locales/en-US/chrome/pippki/pref-validation.dtd
>===================================================================
>@@ -30,50 +30,56 @@
.
.
>+<!ENTITY validation.ocsp.caption                  "OCSP">
>+<!ENTITY validation.ocsp.description              "&brandShortName; can use Online Certificate Status Protocol (OCSP) to verify certificates. Set &brandShortName; to use OCSP as follows:">
>+<!ENTITY disableOCSP.label                        "Do not use OCSP for certificate validation">
>+<!ENTITY disableOCSP.accesskey                    "D">
>+<!ENTITY certOCSP.label                           "Use OCSP to validate only certificates that specify an OCSP service URL">
>+<!ENTITY certOCSP.accesskey                       "U">
>+<!ENTITY proxyOCSP.label                          "Use OCSP to validate all certificates using this URL and signer:">
>+<!ENTITY proxyOCSP.accesskey                      "e">
O is available, any reason why you did not use that?

r= with above comments taken on board.
Attachment #191316 - Flags: review?(bugzilla) → review+
Attachment #191316 - Attachment is obsolete: true
Attachment #191321 - Flags: review?(bugzilla)
Attachment #191321 - Flags: review?(kaie.bugs)
Comment on attachment 191321 [details] [diff] [review]
Updated patch fixing nits and adding other accesskeys needed for FF

>Index: locales/en-US/chrome/pippki/certManager.dtd
>===================================================================
>@@ -30,82 +30,94 @@
.
.
>+<!ENTITY certmgr.details.label                "Certificate Fields">
>+<!ENTITY certmgr.details.accesskey            "F">
>+<!ENTITY certmgr.fields.label                 "Field Value">
>+<!ENTITY certmgr.fields.accesskey             "V">
>+<!ENTITY certmgr.hierarchy.label              "Certificate Hierarchy">
>+<!ENTITY certmgr.hierarchy.accesskey          "r">
Any reason why this is not "C"?

>Index: locales/en-US/chrome/pippki/deviceManager.dtd
>===================================================================
>@@ -30,30 +30,36 @@
.
.
>+<!ENTITY devmgr.button.changeslotname.label     "Change Slot Name">
>+<!ENTITY devmgr.button.changeslotname.accesskey "S">
Any reason why this is not "C"?
.
.
>+<!ENTITY loaddevice.info                        "Enter the information for the module you want to add.">
>+<!ENTITY loaddevice.modname                     "Module Name:">
>+<!ENTITY loaddevice.modname.default             "New PKCS#11 Module">
>+<!ENTITY loaddevice.filename                    "Module filename:">
>+<!ENTITY loaddevice.browse                      "Browse...">
Need some accesskeys for the above.

r= with those changes
Attachment #191321 - Flags: review?(bugzilla) → review+
Attachment #191321 - Attachment is obsolete: true
Attachment #191446 - Flags: review+
Attachment #191321 - Flags: review?(kaie.bugs)
Attachment #191446 - Flags: review?(kaie.bugs)
Attachment #191446 - Flags: review?(kaie.bugs) → review?(kai.engert)
Attachment #189523 - Flags: review?(timeless) → review?(daniel)
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

Checking in
addrbook/prefs/resources/content/pref-addressing.xul;
new revision: 1.56; previous revision: 1.55
addrbook/prefs/resources/content/pref-editdirectories.xul;
new revision: 1.12; previous revision: 1.11
addrbook/prefs/resources/locale/en-US/pref-addressing.dtd;
new revision: 1.20; previous revision: 1.19
addrbook/prefs/resources/locale/en-US/pref-directory.dtd;
new revision: 1.4; previous revision: 1.3
base/prefs/resources/content/pref-labels.xul;
new revision: 1.8; previous revision: 1.7
base/prefs/resources/content/pref-mailnews.xul;
new revision: 1.79; previous revision: 1.78
base/prefs/resources/content/pref-notifications.xul;
new revision: 1.10; previous revision: 1.9
base/prefs/resources/content/pref-offline.xul;
new revision: 1.8; previous revision: 1.7
base/prefs/resources/content/pref-receipts.xul;
new revision: 1.36; previous revision: 1.35
base/prefs/resources/content/pref-viewing_messages.xul;
new revision: 1.48; previous revision: 1.47
base/prefs/resources/locale/en-US/pref-labels.dtd;
new revision: 1.3; previous revision: 1.2
base/prefs/resources/locale/en-US/pref-mailnews.dtd;
new revision: 1.35; previous revision: 1.34
base/prefs/resources/locale/en-US/pref-notifications.dtd;
new revision: 1.6; previous revision: 1.5
base/prefs/resources/locale/en-US/pref-offline.dtd;
new revision: 1.6; previous revision: 1.5
base/prefs/resources/locale/en-US/pref-receipts.dtd;
new revision: 1.13; previous revision: 1.12
base/prefs/resources/locale/en-US/pref-viewing_messages.dtd;
new revision: 1.24; previous revision: 1.23
compose/prefs/resources/content/pref-composing_messages.xul;
new revision: 1.47; previous revision: 1.46
compose/prefs/resources/content/pref-formatting.xul;
new revision: 1.40; previous revision: 1.39
compose/prefs/resources/locale/en-US/pref-composing_messages.dtd;
new revision: 1.24; previous revision: 1.23
compose/prefs/resources/locale/en-US/pref-formatting.dtd;
new revision: 1.17; previous revision: 1.16
done
Attachment #189517 - Attachment description: Updated MailNews patch. Changed a window.title to pref.ldap.window.title. → Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk)
Comment on attachment 190822 [details] [diff] [review]
Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage / Checked into 1.8.1 branch)

Checking in
inspector/resources/content/prefs/pref-inspector.xul; new revision: 1.16;
previous revision: 1.15
inspector/resources/locale/en-US/prefs.dtd; new revision: 1.4; previous
revision: 1.3
content-packs/resources/content/pref-contentpacks.xul; new revision: 1.33;
previous revision: 1.32
content-packs/resources/locale/en-US/pref-contentpacks.dtd; new revision: 1.13;
previous revision: 1.12
wallet/resources/content/pref-passwords.xul; new revision: 1.23; previous
revision: 1.22
wallet/resources/content/pref-wallet.xul; new revision: 1.22; previous
revision: 1.21
wallet/resources/locale/en-US/pref-passwords.dtd; new revision: 1.7; previous
revision: 1.6
wallet/resources/locale/en-US/pref-wallet.dtd; new revision: 1.6; previous
revision: 1.5
done
Attachment #190822 - Attachment description: Nits fixed. Carrying over r+ flag. → Nits fixed. Carrying over r+ flag. (Checked into trunk)
Comment on attachment 190822 [details] [diff] [review]
Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage / Checked into 1.8.1 branch)

Checking in
content/prefs/pref-inspector.xul; new revision: 1.17; previous revision: 1.16
locale/en-US/prefs.dtd; new revision: 1.5; previous revision: 1.4
done
Attachment #190822 - Attachment description: Nits fixed. Carrying over r+ flag. (Checked into trunk) → Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage)
Version: unspecified → Trunk
Comment on attachment 189523 [details] [diff] [review]
Updated Composer patch. (Checked in trunk / 1.8.1 branch)

r=daniel@glazman.org
Attachment #189523 - Flags: review?(daniel) → review+
Attachment #189523 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #189523 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Comment on attachment 189523 [details] [diff] [review]
Updated Composer patch. (Checked in trunk / 1.8.1 branch)

Checking in
composer/content/pref-composer.xul;
new revision: 1.35; previous revision: 1.34
composer/content/pref-editing.xul;
new revision: 1.40; previous revision: 1.39
composer/content/pref-publish.xul;
new revision: 1.7; previous revision: 1.6
composer/content/pref-toolbars.xul;
new revision: 1.4; previous revision: 1.3
locales/en-US/chrome/composer/pref-composer.dtd;
new revision: 1.3; previous revision: 1.2
locales/en-US/chrome/composer/pref-editing.dtd;
new revision: 1.2; previous revision: 1.1
locales/en-US/chrome/composer/pref-publish.dtd;
new revision: 1.2; previous revision: 1.1
locales/en-US/chrome/composer/pref-toolbars.dtd;
new revision: 1.2; previous revision: 1.1
done
Attachment #189523 - Attachment description: Updated Composer patch. → Updated Composer patch. (Checked in)
Comment on attachment 191446 [details] [diff] [review]
What IanN said. Carrying over his r+.

On first look it appeared your changing a lot of text, but after applying your
patch to my tree, and creating a -w version of the patch, I figured you are
now, but 50% of your patch is caused by whitespace changes.

So I'm basing my review on that smaller patch I have created from your changes.

I don't understand why you are renaming a lot of XML string identifiers. This
has the unnecessariy risk you made a mistake while trying to keep both places
of each pair consistent. In addition, IMHO it creates unnecessary additional
work for those who translate Mozilla.

I'm not going to check if you made all those changes consistently. I hope you
have...

All your other changes are about adding access keys. That's great.

r=kaie assuming you or somebody else doublechecks that your identifier renames
do not introduce any regressions.
Attachment #191446 - Flags: review?(kai.engert) → review+
Attachment #191446 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 191446 [details] [diff] [review]
What IanN said. Carrying over his r+.

>     <button id="deleteCrl" disabled="true"
>             label="&validation.deletecrl.label;"
>-            oncommand="DeleteCrlSelected();"/>
>+            oncommand="DeleteCrlSelected();"
>+            accesskey="&validation.deletecrl.accesskey;"/>
>     <button id="editPrefs" class="push" disabled="true"
>             label="&validation.advanced.label;"
>-            oncommand="EditAutoUpdatePrefs();"/>
>+            oncommand="EditAutoUpdatePrefs();"
>+            accesskey="&validation.advanced.accesskey;"/>
>     <button id="updateCRL" class="push" disabled="true"
>             label="&validation.updatecrl.label;"
>-            oncommand="UpdateCRL();"/>
>-
>-    <button id="importCRL" class="push" label="&certmgr.restore.label;" oncommand="ImportCRL();"/>
>+            oncommand="UpdateCRL();"
>+            accesskey="&validation.updatecrl.accesskey;"/>
>+    <button id="importCRL" class="push"
>+            label="&certmgr.restore.label;"
>+            oncommand="ImportCRL();"
>+            accesskey="&certmgr.restore.accesskey;"/>

>         <radio value="0" label="&managepassword.askfirsttime;" id="askFirstTime"
>-               style="margin: 0px;" oncommand="changePasswordSettings(false);"/>
>+               style="margin: 0px;" oncommand="changePasswordSettings(false);"
>+               accesskey="&managepassword.askfirsttime.accesskey;"/>
>         <radio value="1" label="&managepassword.askeverytime;" id="askEveryTime"
>-               style="margin: 0px;" oncommand="changePasswordSettings(false);"/>
>+               style="margin: 0px;" oncommand="changePasswordSettings(false);"
>+               accesskey="&managepassword.askeverytime.accesskey;"/>
>         <hbox align="center">
>           <radio value="2" label="&managepassword.asktimeout;" id="askTimeout"
>-                 style="margin: 0px;" oncommand="changePasswordSettings(true);"/>
>+                 style="margin: 0px;" oncommand="changePasswordSettings(true);"
>+                 accesskey="&managepassword.asktimeout.accesskey;"/>

You should put the accesskeys next to the labels (which would also make the
diff smaller) e.g.
     <button id="deleteCrl" disabled="true"
	     label="&validation.deletecrl.label;"
+	     accesskey="&validation.deletecrl.accesskey;"
	     oncommand="DeleteCrlSelected();"/>
Attachment #191446 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Fixed Neil's nit. Carrying over r/sr, ready for someone to checkin, please.
Attachment #191446 - Attachment is obsolete: true
Attachment #196894 - Flags: superreview+
Attachment #196894 - Flags: review+
Attached patch Per IRC discussion (obsolete) — Splinter Review
Attachment #196894 - Attachment is obsolete: true
Attachment #196924 - Attachment is obsolete: true
Attachment #196928 - Flags: superreview+
Attachment #196928 - Flags: review+
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

The "Updated MailNews patch" has caused at least one regression in Thunderbird.
Open up the Options Dialog, select Composition->Addressing and select "Edit
Directories ..." (next to the directory server). A dialog comes up with:

XML Parsing Error: undefined entity
Location: chrome://messenger/content/addressbook/pref-editdirectories.xul
Line Number 43, Column 1:

Looks like the thunderbird version of the dtd hasn't been updated to match the
mailnews version.
Simple entity name change to fix a regression I introduced in TB, and removal
of an unused entity.
Attachment #197743 - Flags: superreview?(bienvenu)
Attachment #197743 - Flags: review?(bienvenu)
Attachment #197743 - Flags: superreview?(bienvenu)
Attachment #197743 - Flags: superreview+
Attachment #197743 - Flags: review?(bienvenu)
Attachment #197743 - Flags: review+
regression fix patch checked in.
Attachment #197743 - Attachment description: Align TB files to fix regression → Align TB files to fix regression (Checked in)
Comment on attachment 201243 [details] [diff] [review]
Updated patch, with remaining bits to fix bug 302477 added. Not sure if I should ask again for r/sr...

Looks good, but I guess the form/password manager access keys aren't going to work until our access key in tab story is fixed...
Attachment #201243 - Attachment is obsolete: true
Attachment #201271 - Attachment description: Fixed an entity name as requested by Neil on IRC, carrying over r/sr → Fixed an entity name as requested by Neil on IRC, carrying over r/sr (checked in)
What is left is the renaming of DomI pref panels, but not sure how to do that since it's been added to the tree with a lot of localized versions: a patch for en-US only already burnt the tree...
If it's not really important, I guess this bug can be closed.
(In reply to comment #62)
> What is left is the renaming of DomI pref panels, but not sure how to do that
> since it's been added to the tree with a lot of localized versions: a patch for
> en-US only already burnt the tree...
> If it's not really important, I guess this bug can be closed.
> 

--> Considering why this bug was filed in the first place, I think we're fine now. 
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 201271 [details] [diff] [review]
Fixed an entity name as requested by Neil on IRC, carrying over r/sr (Checked in trunk / branch 1.8.1)

Requesting a= for SM1.1a for this polish/accessibility patch
Attachment #201271 - Flags: approval-seamonkey1.1a?
Comment on attachment 201271 [details] [diff] [review]
Fixed an entity name as requested by Neil on IRC, carrying over r/sr (Checked in trunk / branch 1.8.1)

a=me for SeaMonkey 1.1
Attachment #201271 - Flags: approval-seamonkey1.1a? → approval-seamonkey1.1a+
Comment on attachment 190822 [details] [diff] [review]
Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage / Checked into 1.8.1 branch)

Request a= for SM1.1a - more accesskeys/polish
Attachment #190822 - Flags: approval-seamonkey1.1a?
Comment on attachment 190822 [details] [diff] [review]
Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage / Checked into 1.8.1 branch)

a=me for SeaMonkey 1.1
Attachment #190822 - Flags: approval-seamonkey1.1a? → approval-seamonkey1.1a+
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

Requesting a= for SM1.1a for more polish
Attachment #189517 - Flags: approval-seamonkey1.1a?
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

Requesting a= for branch 1.8.1 for shared TB pref code (pref-editdirectories.xul) and relevant changes to locale dtd file.
Attachment #189517 - Flags: approval-branch-1.8.1?(mscott)
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

a=me for SeaMonkey 1.1 (for suite-specifc changes)
Attachment #189517 - Flags: approval-seamonkey1.1a? → approval-seamonkey1.1a+
Attachment #189517 - Flags: approval-branch-1.8.1?(mscott) → approval-branch-1.8.1+
Comment on attachment 189523 [details] [diff] [review]
Updated Composer patch. (Checked in trunk / 1.8.1 branch)

requesting a= for SM1.1a for sync/polish.
Attachment #189523 - Flags: approval-seamonkey1.1a?
Comment on attachment 189517 [details] [diff] [review]
Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)

Checking in (branch 1.8.1)
mailnews/addrbook/prefs/resources/content/pref-addressing.xul;
new revision: 1.55.2.1; previous revision: 1.55
mailnews/addrbook/prefs/resources/content/pref-editdirectories.xul;
new revision: 1.11.8.1; previous revision: 1.11
mailnews/addrbook/prefs/resources/locale/en-US/pref-addressing.dtd;
new revision: 1.19.2.1; previous revision: 1.19
mailnews/addrbook/prefs/resources/locale/en-US/pref-directory.dtd;
new revision: 1.3.28.1; previous revision: 1.3
mailnews/base/prefs/resources/content/pref-labels.xul;
new revision: 1.7.12.1; previous revision: 1.7
mailnews/base/prefs/resources/content/pref-mailnews.xul;
new revision: 1.78.4.1; previous revision: 1.78
mailnews/base/prefs/resources/content/pref-notifications.xul;
new revision: 1.9.22.1; previous revision: 1.9
mailnews/base/prefs/resources/content/pref-offline.xul;
new revision: 1.7.12.1; previous revision: 1.7
mailnews/base/prefs/resources/content/pref-receipts.xul;
new revision: 1.35.26.1; previous revision: 1.35
mailnews/base/prefs/resources/content/pref-viewing_messages.xul;
new revision: 1.47.6.1; previous revision: 1.47
mailnews/base/prefs/resources/locale/en-US/pref-labels.dtd;
new revision: 1.2.140.1; previous revision: 1.2
mailnews/base/prefs/resources/locale/en-US/pref-mailnews.dtd;
new revision: 1.34.4.1; previous revision: 1.34
mailnews/base/prefs/resources/locale/en-US/pref-notifications.dtd;
new revision: 1.5.28.1; previous revision: 1.5
mailnews/base/prefs/resources/locale/en-US/pref-offline.dtd;
new revision: 1.5.28.1; previous revision: 1.5
mailnews/base/prefs/resources/locale/en-US/pref-receipts.dtd;
new revision: 1.12.28.1; previous revision: 1.12
mailnews/base/prefs/resources/locale/en-US/pref-viewing_messages.dtd;
new revision: 1.23.6.1; previous revision: 1.23
mailnews/compose/prefs/resources/content/pref-composing_messages.xul;
new revision: 1.46.4.1; previous revision: 1.46
mailnews/compose/prefs/resources/content/pref-formatting.xul;
new revision: 1.39.78.1; previous revision: 1.39
mailnews/compose/prefs/resources/locale/en-US/pref-composing_messages.dtd;
new revision: 1.23.4.1; previous revision: 1.23
mailnews/compose/prefs/resources/locale/en-US/pref-formatting.dtd;
new revision: 1.16.20.1; previous revision: 1.16
mail/locales/en-US/chrome/messenger/addressbook/pref-directory.dtd;
new revision: 1.3.4.1; previous revision: 1.3
done
Attachment #189517 - Attachment description: Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk) → Updated MailNews patch. Changed a window.title to pref.ldap.window.title. (Checked into trunk / 1.8.1 branch)
Attachment #197743 - Attachment description: Align TB files to fix regression (Checked in) → Align TB files to fix regression (Checked in trunk / 1.8.1 branch)
Attachment #189523 - Flags: approval-seamonkey1.1a? → approval-seamonkey1.1a+
Comment on attachment 190822 [details] [diff] [review]
Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage / Checked into 1.8.1 branch)

Checking in (branch 1.8.1)
content-packs/resources/content/pref-contentpacks.xul;
new revision: 1.32.2.1; previous revision: 1.32
content-packs/resources/locale/en-US/pref-contentpacks.dtd;
new revision: 1.12.80.1; previous revision: 1.12
wallet/resources/content/pref-passwords.xul;
new revision: 1.22.8.1; previous revision: 1.22
wallet/resources/content/pref-wallet.xul;
new revision: 1.21.8.1; previous revision: 1.21
wallet/resources/locale/en-US/pref-passwords.dtd;
new revision: 1.6.148.1; previous revision: 1.6
wallet/resources/locale/en-US/pref-wallet.dtd;
new revision: 1.5.28.1; previous revision: 1.5
done
Attachment #190822 - Attachment description: Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage) → Nits fixed. Carrying over r+ flag. (Checked into trunk - inspector parts now backed out to fix bustage / Checked into 1.8.1 branch)
Comment on attachment 189523 [details] [diff] [review]
Updated Composer patch. (Checked in trunk / 1.8.1 branch)

Checking in (branch 1.8.1)
composer/content/pref-composer.xul;
new revision: 1.34.8.1; previous revision: 1.34
composer/content/pref-editing.xul;
new revision: 1.39.28.1; previous revision: 1.39
composer/content/pref-publish.xul;
new revision: 1.6.28.1; previous revision: 1.6
composer/content/pref-toolbars.xul;
new revision: 1.3.70.1; previous revision: 1.3
locales/en-US/chrome/composer/pref-composer.dtd;
new revision: 1.2.8.1; previous revision: 1.2
locales/en-US/chrome/composer/pref-editing.dtd;
new revision: 1.1.8.1; previous revision: 1.1
locales/en-US/chrome/composer/pref-publish.dtd;
new revision: 1.1.8.1; previous revision: 1.1
locales/en-US/chrome/composer/pref-toolbars.dtd;
new revision: 1.1.8.1; previous revision: 1.1
done
Attachment #189523 - Attachment description: Updated Composer patch. (Checked in) → Updated Composer patch. (Checked in trunk / 1.8.1 branch)
Comment on attachment 201271 [details] [diff] [review]
Fixed an entity name as requested by Neil on IRC, carrying over r/sr (Checked in trunk / branch 1.8.1)

Requesting a= for polish / accessibility patch for 1.8.1 branch
Attachment #201271 - Flags: approval-branch-1.8.1?(wtchang)
Attachment #201271 - Flags: approval-branch-1.8.1?(wtchang) → approval-branch-1.8.1?(kengert)
Attachment #201271 - Flags: approval-branch-1.8.1?(kengert) → approval-branch-1.8.1+
Comment on attachment 201271 [details] [diff] [review]
Fixed an entity name as requested by Neil on IRC, carrying over r/sr (Checked in trunk / branch 1.8.1)

Checking in (branch 1.8.1)
security/manager/pki/resources/content/CAOverlay.xul;
new revision: 1.25.28.1; previous revision: 1.25
security/manager/pki/resources/content/crlManager.xul;
new revision: 1.21.6.1; previous revision: 1.21
security/manager/pki/resources/content/device_manager.xul;
new revision: 1.18.8.1; previous revision: 1.18
security/manager/pki/resources/content/load_device.xul;
new revision: 1.7.18.1; previous revision: 1.7
security/manager/pki/resources/content/MineOverlay.xul;
new revision: 1.25.28.1; previous revision: 1.25
security/manager/pki/resources/content/OthersOverlay.xul;
new revision: 1.14.68.1; previous revision: 1.14
security/manager/pki/resources/content/pref-certs.xul;
new revision: 1.13.8.1; previous revision: 1.13
security/manager/pki/resources/content/pref-masterpass.xul;
new revision: 1.22.8.1; previous revision: 1.22
security/manager/pki/resources/content/pref-ssl.xul;
new revision: 1.27.8.1; previous revision: 1.27
security/manager/pki/resources/content/pref-validation.xul;
new revision: 1.16.8.1; previous revision: 1.16
security/manager/pki/resources/content/WebSitesOverlay.xul;
new revision: 1.23.28.1; previous revision: 1.23
security/manager/locales/en-US/chrome/pippki/certManager.dtd;
new revision: 1.1.8.1; previous revision: 1.1
security/manager/locales/en-US/chrome/pippki/deviceManager.dtd;
new revision: 1.1.8.1; previous revision: 1.1
security/manager/locales/en-US/chrome/pippki/pref-masterpass.dtd;
new revision: 1.1.8.2; previous revision: 1.1.8.1
security/manager/locales/en-US/chrome/pippki/pref-security.dtd;
new revision: 1.1.8.1; previous revision: 1.1
security/manager/locales/en-US/chrome/pippki/pref-ssl.dtd;
new revision: 1.1.8.1; previous revision: 1.1
security/manager/locales/en-US/chrome/pippki/pref-validation.dtd;
new revision: 1.1.8.1; previous revision: 1.1
extensions/wallet/signonviewer/resources/content/SignonViewer.xul;
new revision: 1.8.8.1; previous revision: 1.8
extensions/wallet/signonviewer/resources/locale/en-US/SignonViewer.dtd;
new revision: 1.5.44.1; previous revision: 1.5
done
Attachment #201271 - Attachment description: Fixed an entity name as requested by Neil on IRC, carrying over r/sr (checked in) → Fixed an entity name as requested by Neil on IRC, carrying over r/sr (Checked in trunk / branch 1.8.1)
You need to log in before you can comment on or make changes to this bug.