+++ This bug was initially created as a clone of Bug #461112 +++ Using an optimized version (from Philipp Kewisch) of the script mentioned in http://www.chrisfinke.com/2008/04/22/finding-unused-entities-in-your-firefox-extensions/ I was able to identify lots of unused strings in editor/ui/locales Those should be removed to lessen the localization burden for new localizers and to reduce the build size. CC'ing philor and KaiRo as likely reviewers, since this patch also touches a SeaMonkey file. The attached patch removes 98 strings on the Thunderbird side and 74 on the SeaMonkey side. I'll attach a patch that only deals with .dtd files first. A .properties patch will follow later.

Comment on attachment 347121 [details] [diff] [review] Obsolete strings in editor/ui Asking for a second review from KaiRo, as this patch also touches one SeaMonkey file.

Comment on attachment 347121 [details] [diff] [review] Obsolete strings in editor/ui Looks like the copy-paste into debugQAEditorOverlay.dtd broke the UTF-8 ellipses. Other than that, I'm happy to see unused strings go away, but I don't think I own these. I've always treated editor/ui as r+sr=neil, since he's pretty much the only active editor peer.

Comment on attachment 347121 [details] [diff] [review] Obsolete strings in editor/ui As KaiRo knows, I played with this script a bit lately, here are my nits: <!-- For a "Paste" submenu when more than 1 clipboard formats are available --> -<!ENTITY pasteHTMLCmd.label "HTML"> -<!ENTITY pasteHTMLCmd.accesskey "H"> <!ENTITY pasteTextCmd.label "Text"> <!ENTITY pasteTextCmd.accesskey "T"> <!ENTITY pasteImageCmd.label "Image"> <!ENTITY pasteImageCmd.accesskey "I"> <!ENTITY pasteRowsCmd.label "Rows"> <!ENTITY pasteRowsCmd.accesskey "R"> <!ENTITY pasteColumnsCmd.label "Columns"> <!ENTITY pasteColumnsCmd.accesskey "C"> All these can go out as they are not implemented, see http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/editorOverlay.xul#430 We should remove the code too. And with this we can remove also InitPasteAsMenu function from http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.js#2001 +++ b/editor/ui/locales/en-US/chrome/composer/pref-composer.dtd @@ -1,39 +1,27 @@ <!-- extracted from content/pref-composer.xul --> <!--LOCALIZATION NOTE : FILE 'Composer' prefs dialog. Similar to Communcator 4.x Document Properties/Colors and Background --> <!--LOCALIZATION NOTE (pref.composer.title): DONT_TRANSLATE --> <!ENTITY pref.composer.title "Composer"> - <!ENTITY saving "Saving"> -<!ENTITY AutoSaveCheck "Automatically save every"> -<!ENTITY minText "minutes"> - -<!ENTITY exterLegend.label "External Editors"> <!ENTITY htmlSource "HTML Source:"> -<!ENTITY imageeditor "Images:"> -<!ENTITY chooseButton.label "Choose"> saving and htmlSource are obsolete too. +++ b/editor/ui/locales/en-US/chrome/dialogs/EditorPublish.dtd @@ -42,22 +42,19 @@ <!ENTITY publishTab.label "Publish"> <!ENTITY settingsTab.label "Settings"> <!ENTITY publishButton.label "Publish"> <!-- Publish Tab Panel --> <!ENTITY siteList.label "Site Name:"> -<!ENTITY siteList.accesskey "e"> -<!ENTITY siteList.tooltip "Choose the site that you want to publish to"> <!ENTITY newSiteButton.label "New Site"> <!ENTITY newSiteButton.accesskey "N"> <!ENTITY docDirList.label "Site subdirectory for this page:"> -<!ENTITY docDirList.accesskey "S"> <!ENTITY docDirList.tooltip "Choose or enter the name of the remote subdirectory for this page"> We should rather use them, like this: > diff --git a/editor/ui/dialogs/content/EditorPublish.xul b/editor/ui/dialogs/content/EditorPublish.xul > --- a/editor/ui/dialogs/content/EditorPublish.xul > +++ b/editor/ui/dialogs/content/EditorPublish.xul > @@ -76,10 +76,13 @@ > <columns><column/><column/><column/></columns> > <rows> > <row align="center"> > - <label value="&siteList.label;"/> > + <label value="&siteList.label;" > + accesskey="&siteList.accesskey;" > + control="SiteList"/> > <!-- Contents filled in at runtime --> > <menulist id="SiteList" > style="min-width:18em; max-width:18em;" crop="right" > + tooltiptext="&siteList.tooltip;" > oncommand="SelectSiteList();"/> > <hbox> > <button label="&newSiteButton.label;" > @@ -106,7 +109,9 @@ > </rows> > </grid> > <spacer class="spacer"/> > - <label value="&docDirList.label;"/> > + <label value="&docDirList.label;" > + accesskey="&docDirList.accesskey;" > + control="DocDirList"/> > <hbox align="center"> > <!-- Contents filled in at runtime --> > <menulist id="DocDirList" class="minWidth20 uri-element" editable="true" flex="1" <!ENTITY password.label "Password:"> <!ENTITY password.accesskey "w"> -<!ENTITY password.tooltip "The password associated with your user name"> <!ENTITY savePassword.label "Save Password"> The same here with password.tooltip, use this f.e.: > +++ b/editor/ui/dialogs/content/EditorPublishOverlay.xul > @@ -82,7 +82,8 @@ > control="PasswordInput"/> > <hbox> > <textbox id="PasswordInput" type="password" class="MinWidth5em" > - oninput="onInputSettings();"/> > + oninput="onInputSettings();" > + tooltiptext="&password.tooltip;"/> > <checkbox id="SavePassword" label="&savePassword.label;" > accesskey="&savePassword.accesskey;" > tooltiptext="&savePassword.tooltip;" And take care of patch encoding, this one is in CP1250.

New patch that incorporates the comments from Phil and Wladow. I'm asking Neil for review, since editor/ seems to be his domain and the code affects both Thunderbird and SeaMonkey.

Comment on attachment 347155 [details] [diff] [review] Obsolete strings in editor/ui - dtd part - v2 >diff --git a/editor/ui/locales/en-US/chrome/composer/editorNavigatorOverlay.dtd b/editor/ui/locales/en-US/chrome/composer/editorNavigatorOverlay.dtd >--- a/editor/ui/locales/en-US/chrome/composer/editorNavigatorOverlay.dtd >+++ b/editor/ui/locales/en-US/chrome/composer/editorNavigatorOverlay.dtd >@@ -1,7 +1,2 @@ >- >-<!ENTITY editPageCmd.label "Edit Page in Composer"> >-<!ENTITY editPageCmd.accesskey "e"> >- > <!ENTITY editLinkCmd.label "Edit Link in Composer"> > <!ENTITY editLinkCmd.accesskey "E"> >- We don't use this version either, we use the one in editorOverlay.dtd

Comment on attachment 347155 [details] [diff] [review] Obsolete strings in editor/ui - dtd part - v2 I saw an entity for an unused inline spellchecking preference. I'll need to see if it's likely that we might want such a preference. >+<!ENTITY statusText.label "Done loading page"> Not used? >+<!ENTITY insertFormMenu.label "Form"> I don't really want to move the form menu entities here, I want to be able to move the form menu from debug to Insert... >+<!ENTITY NormalAbbr.label "Text"> >+<!ENTITY ParagraphAbbr.label "P"> >+<!ENTITY Heading1Abbr.label "H1"> >+<!ENTITY Heading2Abbr.label "H2"> >+<!ENTITY Heading3Abbr.label "H3"> >+<!ENTITY Heading4Abbr.label "H4"> >+<!ENTITY Heading5Abbr.label "H5"> >+<!ENTITY Heading6Abbr.label "H6"> >+<!ENTITY BlockquoteAbbr.label "BQ"> >+<!ENTITY AddressAbbr.label "Addr."> >+<!ENTITY PreformatAbbr.label "Pre."> Not used?

(In reply to comment #5) > >--- a/editor/ui/locales/en-US/chrome/composer/editorNavigatorOverlay.dtd > >+++ b/editor/ui/locales/en-US/chrome/composer/editorNavigatorOverlay.dtd > >@@ -1,7 +1,2 @@ > >- > >-<!ENTITY editPageCmd.label "Edit Page in Composer"> > >-<!ENTITY editPageCmd.accesskey "e"> > >- > > <!ENTITY editLinkCmd.label "Edit Link in Composer"> > > <!ENTITY editLinkCmd.accesskey "E"> > >- > We don't use this version either, we use the one in editorOverlay.dtd Right. I'll add this to a v3 patch. (In reply to comment #6) > >+<!ENTITY statusText.label "Done loading page"> > Not used? See http://mxr.mozilla.org/comm-central/search?string=statusText.label&tree=comm-central Only debugQATextEditorShell.xul uses this string right now, which is why I moved it from shared editor.dtd to Suite-only debugQAEditorOverlay.dtd. > >+<!ENTITY insertFormMenu.label "Form"> > I don't really want to move the form menu entities here, I want to be able to > move the form menu from debug to Insert... Can we please do such stuff in a followup bug? I'd really like to focus on cleanup work in this bug. > >+<!ENTITY NormalAbbr.label "Text"> > >+<!ENTITY ParagraphAbbr.label "P"> > >+<!ENTITY Heading1Abbr.label "H1"> > >+<!ENTITY Heading2Abbr.label "H2"> > >+<!ENTITY Heading3Abbr.label "H3"> > >+<!ENTITY Heading4Abbr.label "H4"> > >+<!ENTITY Heading5Abbr.label "H5"> > >+<!ENTITY Heading6Abbr.label "H6"> > >+<!ENTITY BlockquoteAbbr.label "BQ"> > >+<!ENTITY AddressAbbr.label "Addr."> > >+<!ENTITY PreformatAbbr.label "Pre."> > Not used? You're right. According to http://mxr.mozilla.org/comm-central/search?string=Abbr.label&tree=comm-central those strings aren't used at all. My bad. I'll remove them completely in the v3 patch.

Sorry, no update on the inline spellchecking preference entities yet. (In reply to comment #7) > (In reply to comment #6) > > >+<!ENTITY statusText.label "Done loading page"> > > Not used? > Only debugQATextEditorShell.xul uses this string right now, which is why I > moved it from shared editor.dtd to Suite-only debugQAEditorOverlay.dtd. Sorry, I hadn't noticed it there. > > >+<!ENTITY insertFormMenu.label "Form"> > > I don't really want to move the form menu entities here, I want to be able to > > move the form menu from debug to Insert... > Can we please do such stuff in a followup bug? I'd really like to focus on > cleanup work in this bug. I just want to be *able* to move the form menu... which means not (re)moving those entities please.

(In reply to comment #9) >>+<!ENTITY insertFormMenu.label "Form"> > > I just want to be *able* to move the form menu... which means not > (re)moving those entities please. What is hindering you to move those strings wherever you want them when they are in debugQAEditorOverlay.dtd instead of editorOverlay.dtd? But I can surely move them to a more appropriate place if you tell me where to put them. I just don't want them lying around unused anymore. Such things have a tendency to become permanent as evidenced by the huge amount of unused strings here, in bug 463886 or in bug 461112.

Comment on attachment 347190 [details] [diff] [review] Obsolete strings in editor/ui - dtd part - v3 > <!ENTITY printPreviewCmd.label "Print Preview"> Looks like this isn't really used either (commented out in the XUL).

(In reply to comment #10) >(In reply to comment #9) >>>+<!ENTITY insertFormMenu.label "Form"> >> >> I just want to be *able* to move the form menu... which means not >> (re)moving those entities please. >What is hindering you to move those strings wherever you want them when >they are in debugQAEditorOverlay.dtd instead of editorOverlay.dtd? Because the place I would want to move them would be back to editorOverlay.dtd!

Comment on attachment 347155 [details] [diff] [review] Obsolete strings in editor/ui - dtd part - v2 >-<!ENTITY spellCheckInline.label "Check spelling as you type"> >-<!ENTITY spellCheckInline.accesskey "C"> As with the forms, I'd like to describe this as a potential feature rather than an obsolete string.

(In reply to comment #11) >> <!ENTITY printPreviewCmd.label "Print Preview"> > > Looks like this isn't really used either (commented out in the XUL). I'll remove it.

(In reply to comment #12 and comment #13) >>>>+<!ENTITY insertFormMenu.label "Form"> > >>What is hindering you to move those strings wherever you want them when >>they are in debugQAEditorOverlay.dtd instead of editorOverlay.dtd? > >Because the place I would want to move them would be back to >editorOverlay.dtd! > >>-<!ENTITY spellCheckInline.label "Check spelling as you type"> >>-<!ENTITY spellCheckInline.accesskey "C"> > > As with the forms, I'd like to describe this as a potential feature rather > than an obsolete string. Given our really bad history with regards to not removing obsolete strings consistently (see this bug, bug 463886 and bug 461112), I'm really opposed to letting those strings stay in the products as is. It's really only a small matter to add them back, *if* a potential feature or bugfix becomes a reality. So I'd ask you to reconsider this. Please keep in mind that every additional string costs localization time for a new localizer. And these guys don't have much time to spare. As localizing Firefox is their obvious number one priority, Thunderbird only comes in second place with SeaMonkey and Sunbird fighting for a distant third place. My goal for Thunderbird is to reach over 50 supported locales in 2009 (from 46 at the moment) and I know that KaiRo hopes to expand the number of SeaMonkey localizations significantly as well (from the current 16). So this (additional unneeded work for new localizers) is a real concern here.

This patch covers all review comments until comment 11. In addition I also did the following in editorOverlay.xul: - some minor whitespace cleanup - removed two more commented out code sections - removed the relevant strings (insertBreakCmd.accesskey and insertBreakCmd.label) in case they weren't used in another location (as with size-xx-smallCmd.label and size-xx-smallCmd.accesskey) The patch does not yet cover the issues raised in comment 12 and comment 13 for the reasons laid out in comment 15.

Comment on attachment 347214 [details] [diff] [review] Obsolete strings in editor/ui - dtd part - v4 Ugh, totally forgot to ask for review. Please see comment 15 and comment 16 for more info on this patch.

I'm very much for removing strings if there's not a bug+patch out there for actually using them. As for the stuff to be moved to debugQA, I think if the overlay/menuitems are in debugQA, the strings for them should be as well, or else it's even harder for people working on the code to find where the fitting strings are.

Comment on attachment 347214 [details] [diff] [review] Obsolete strings in editor/ui - dtd part - v4 Oh well, it's not as if any of this stuff has any useful blame.

Hmm, does this mean that debugQA doesn't depend on editorOverlay.dtd any more?

(In reply to comment #20) > Hmm, does this mean that debugQA doesn't depend on editorOverlay.dtd any more? Given that debugQAEditorOverlay.xul references 45 strings and the patch adds 24 strings to the 21 existing ones in debugQAEditorOverlay.dtd, the .xul file should no longer be dependent on editorOverlay.dtd. I'll test this before checkin. If nothing breaks, I'll remove the reference to editorOverlay.dtd in debugQAEditorOverlay.xul as well.

Changeset 2fae120cb481 checked in with reference to editorOverlay.dtd removed in debugQAEditorOverlay.xul. To make this work however, I had to duplicate the two strings <!ENTITY pasteAsQuotationCmd.label "Paste As Quotation"> <!ENTITY pasteAsQuotationCmd.accesskey "Q"> from editorOverlay.dtd in debugQAEditorOverlay.dtd Neil, thanks for the review and the consideration of comments here.

Here's a much smaller patch to get rid of unused strings in one .properties file (editor.properties).

Comment on attachment 347370 [details] [diff] [review] Obsolete strings in editor/ui - properties part >-Yes=Yes > No=No > Save=Save > DontSave=Don't Save > More=More > Fewer=Fewer > Less=Less I'm sure we can get rid of more of these. I couldn't find "DontSave" or "Fewer" for instance. > # LOCALIZATION NOTE (TableSelectKey): DONT_TRANSLATE > TableSelectKey=Ctrl+ >-# LOCALIZATION NOTE (XulKeyDefault): DONT_TRANSLATE >-XulKeyDefault=Ctrl+ > # LOCALIZATION NOTE (XulKeyMac): DONT_TRANSLATE > XulKeyMac=Cmd+ >-# LOCALIZATION NOTE (XulKeyUnix): DONT_TRANSLATE >-XulKeyUnix=Alt+ > # LOCALIZATION NOTE (Del): DONT_TRANSLATE > Del=Del Surely these should in fact be translated (e.g. Strg+ in German)?

Changeset 8d4708360005 pushed. -> FIXED